X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/e696a09cd482ddedeec742127297e90287012356..ba34a22d9918ae97306472c04701e69090821c82:/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 90de800b2f..efc61eee8c 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -14,11 +14,21 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   def with_container_auth(ctr)
     auth_was = Thread.current[:api_client_authorization]
-    Thread.current[:api_client_authorization] = ApiClientAuthorization.find_by_uuid(ctr.auth_uuid)
+    client_was = Thread.current[:api_client]
+    token_was = Thread.current[:token]
+    user_was = Thread.current[:user]
+    auth = ApiClientAuthorization.find_by_uuid(ctr.auth_uuid)
+    Thread.current[:api_client_authorization] = auth
+    Thread.current[:api_client] = auth.api_client
+    Thread.current[:token] = auth.token
+    Thread.current[:user] = auth.user
     begin
       yield
     ensure
       Thread.current[:api_client_authorization] = auth_was
+      Thread.current[:api_client] = client_was
+      Thread.current[:token] = token_was
+      Thread.current[:user] = user_was
     end
   end
 
@@ -56,6 +66,18 @@ class ContainerRequestTest < ActiveSupport::TestCase
     end
   end
 
+  def configure_preemptible_instance_type
+    Rails.configuration.InstanceTypes = ConfigLoader.to_OrderedOptions({
+      "a1.small.pre" => {
+        "Preemptible" => true,
+        "Price" => 0.1,
+        "ProviderType" => "a1.small",
+        "VCPUs" => 1,
+        "RAM" => 1000000000,
+      },
+    })
+  end
+
   test "Container request create" do
     set_user_from_auth :active
     cr = create_minimal_req!
@@ -153,7 +175,10 @@ 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_equal 0, Rails.configuration.Containers.DefaultKeepCacheRAM
+    assert_equal 8589934592, Rails.configuration.Containers.DefaultKeepCacheDisk
 
     assert_not_nil cr.container_uuid
     c = Container.find_by_uuid cr.container_uuid
@@ -164,7 +189,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_disk"=>8589934592, "keep_cache_ram"=>0, "vcpus" => 2, "ram" => 30}.to_a - c.runtime_constraints.to_a).empty?
     assert_operator 0, :<, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do
@@ -209,11 +234,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     act_as_system_user do
       Container.find_by_uuid(cr.container_uuid).
-        update_attributes!(state: Container::Cancelled)
+        update_attributes!(state: Container::Cancelled, cost: 1.25)
     end
 
     cr.reload
     assert_equal "Final", cr.state
+    assert_equal 1.25, cr.cumulative_cost
     assert_equal users(:active).uuid, cr.modified_by_user_uuid
   end
 
@@ -239,12 +265,14 @@ class ContainerRequestTest < ActiveSupport::TestCase
     log_pdh = 'fa7aeb5140e2848d39b416daeef4ffc5+45'
     act_as_system_user do
       c.update_attributes!(state: Container::Complete,
+                           cost: 1.25,
                            output: output_pdh,
                            log: log_pdh)
     end
 
     cr.reload
     assert_equal "Final", cr.state
+    assert_equal 1.25, cr.cumulative_cost
     assert_equal users(:active).uuid, cr.modified_by_user_uuid
 
     assert_not_nil cr.output_uuid
@@ -333,7 +361,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     cr2 = with_container_auth(c) do
       create_minimal_req!(priority: 10, state: "Committed", container_count_max: 1, command: ["echo", "foo2"])
     end
-    assert_not_nil cr2.requesting_container_uuid
+    assert_equal c.uuid, cr2.requesting_container_uuid
     assert_equal users(:active).uuid, cr2.modified_by_user_uuid
 
     c2 = Container.find_by_uuid cr2.container_uuid
@@ -447,10 +475,31 @@ class ContainerRequestTest < ActiveSupport::TestCase
   ].each do |token, expected, expected_priority|
     test "create as #{token} and expect requesting_container_uuid to be #{expected}" do
       set_user_from_auth token
-      cr = ContainerRequest.create(container_image: "img", output_path: "/tmp", command: ["echo", "foo"])
+      cr = create_minimal_req!
+      assert_not_nil cr.uuid, 'uuid should be set for newly created container_request'
+      assert_equal expected, cr.requesting_container_uuid
+      assert_equal expected_priority, cr.priority
+    end
+  end
+
+  [
+    ['running_container_auth', 'zzzzz-dz642-runningcontainr', 501],
+  ].each do |token, expected, expected_priority|
+    test "create as #{token} with requesting_container_uuid set and expect output to be intermediate" do
+      set_user_from_auth token
+      cr = create_minimal_req!
       assert_not_nil cr.uuid, 'uuid should be set for newly created container_request'
       assert_equal expected, cr.requesting_container_uuid
       assert_equal expected_priority, cr.priority
+
+      cr.state = ContainerRequest::Committed
+      cr.save!
+
+      run_container(cr)
+      cr.reload
+      output = Collection.find_by_uuid(cr.output_uuid)
+      props = {"type": "intermediate", "container_request": cr.uuid}
+      assert_equal props.symbolize_keys, output.properties.symbolize_keys
     end
   end
 
@@ -745,6 +794,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     prev_container_uuid = cr.container_uuid
 
     act_as_system_user do
+      c.update_attributes!(cost: 0.5, subrequests_cost: 1.25)
       c.update_attributes!(state: Container::Cancelled)
     end
 
@@ -757,6 +807,9 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     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.update_attributes!(cost: 0.125)
       c.update_attributes!(state: Container::Cancelled)
       c
     end
@@ -766,6 +819,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal "Final", cr.state
     assert_equal prev_container_uuid, cr.container_uuid
     assert_not_equal cr2.container_uuid, cr.container_uuid
+    assert_equal 1.875, cr.cumulative_cost
   end
 
   test "Retry on container cancelled with runtime_token" do
@@ -950,94 +1004,120 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   [
-    [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
-      sp = {"preemptible" => true}
-      common_attrs = {cwd: "test",
-                      priority: 1,
-                      command: ["echo", "hello"],
-                      output_path: "test",
-                      scheduling_parameters: sp,
-                      mounts: {"test" => {"kind" => "json"}}}
-      Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf
+    # client requests preemptible, but types are not configured
+    [false, false, false, true, ActiveRecord::RecordInvalid],
+    [true, false, false, true, ActiveRecord::RecordInvalid],
+    # client requests preemptible, types are configured
+    [false, true, false, true, true],
+    [true, true, false, true, true],
+    # client requests non-preemptible for top-level container
+    [false, false, false, false, false],
+    [true, false, false, false, false],
+    [false, true, false, false, false],
+    [true, true, false, false, false],
+    # client requests non-preemptible for child container, preemptible
+    # is enabled anyway if AlwaysUsePreemptibleInstances and instance types
+    # are configured.
+    [false, false, true, false, false],
+    [true, false, true, false, false],
+    [false, true, true, false, false],
+    [true, true, true, false, true],
+  ].each do |use_preemptible, have_preemptible, is_child, ask, expect|
+    test "with AlwaysUsePreemptibleInstances=#{use_preemptible} and preemptible types #{have_preemptible ? '' : 'not '}configured, create #{is_child ? 'child' : 'top-level'} container request with preemptible=#{ask} and expect #{expect}" do
+      Rails.configuration.Containers.AlwaysUsePreemptibleInstances = use_preemptible
+      if have_preemptible
+        configure_preemptible_instance_type
+      end
+      common_attrs = {
+        cwd: "test",
+        priority: 1,
+        command: ["echo", "hello"],
+        output_path: "test",
+        scheduling_parameters: {"preemptible" => ask},
+        mounts: {"test" => {"kind" => "json"}},
+      }
       set_user_from_auth :active
 
-      cr = create_minimal_req!(common_attrs)
+      if is_child
+        cr = with_container_auth(containers(:running)) do
+          create_minimal_req!(common_attrs)
+        end
+      else
+        cr = create_minimal_req!(common_attrs)
+      end
+
+      cr.reload
       cr.state = ContainerRequest::Committed
 
-      if !expected.nil?
-        assert_raises(expected) do
+      if expect == true || expect == false
+        cr.save!
+        assert_equal expect, cr.scheduling_parameters["preemptible"]
+      else
+        assert_raises(expect) do
           cr.save!
         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"}}}
+  test "config update does not flip preemptible flag on already-committed container requests" do
+    parent = containers(:running_container_with_logs)
+    attrs_p = {
+      scheduling_parameters: {"preemptible" => true},
+      "state" => "Committed",
+      "priority" => 1,
+    }
+    attrs_nonp = {
+      scheduling_parameters: {"preemptible" => false},
+      "state" => "Committed",
+      "priority" => 1,
+    }
+    expect = {false => [], true => []}
 
-      Rails.configuration.Containers.UsePreemptibleInstances = true
-      set_user_from_auth :active
+    with_container_auth(parent) do
+      configure_preemptible_instance_type
+      Rails.configuration.Containers.AlwaysUsePreemptibleInstances = false
 
-      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)
-      end
+      expect[true].push create_minimal_req!(attrs_p)
+      expect[false].push create_minimal_req!(attrs_nonp)
 
-      cr.state = ContainerRequest::Committed
-      cr.save!
+      Rails.configuration.Containers.AlwaysUsePreemptibleInstances = true
 
-      assert_equal false, cr.scheduling_parameters['preemptible']
-    end
-  end
+      expect[true].push create_minimal_req!(attrs_p)
+      expect[true].push create_minimal_req!(attrs_nonp)
+      commit_later = create_minimal_req!()
 
-  [
-    [true, 'zzzzz-dz642-runningcontainr', true],
-    [true, nil, nil],
-    [false, 'zzzzz-dz642-runningcontainr', nil],
-    [false, nil, nil],
-  ].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
-      common_attrs = {cwd: "test",
-                      priority: 1,
-                      command: ["echo", "hello"],
-                      output_path: "test",
-                      mounts: {"test" => {"kind" => "json"}}}
+      Rails.configuration.InstanceTypes = ConfigLoader.to_OrderedOptions({})
 
-      Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf
-      set_user_from_auth :active
+      expect[false].push create_minimal_req!(attrs_nonp)
 
-      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)
+      # Even though preemptible is not allowed, we should be able to
+      # commit a CR that was created earlier when preemptible was the
+      # default.
+      commit_later.update_attributes!(priority: 1, state: "Committed")
+      expect[false].push commit_later
+    end
+
+    set_user_from_auth :active
+    [false, true].each do |pflag|
+      expect[pflag].each do |cr|
+        cr.reload
+        assert_equal pflag, cr.scheduling_parameters['preemptible']
       end
+    end
 
-      cr.state = ContainerRequest::Committed
-      cr.save!
+    act_as_system_user do
+      # Cancelling the parent used to fail while updating the child
+      # containers' priority, because the child containers' unchanged
+      # preemptible fields caused validation to fail.
+      parent.update_attributes!(state: 'Cancelled')
 
-      assert_equal schedule_preemptible, cr.scheduling_parameters['preemptible']
+      [false, true].each do |pflag|
+        expect[pflag].each do |cr|
+          cr.reload
+          assert_equal 0, cr.priority, "unexpected non-zero priority #{cr.priority} for #{cr.uuid}"
+        end
+      end
     end
   end
 
@@ -1068,17 +1148,18 @@ 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
   end
 
-  test "Having preemptible_instances=true create a committed child container request and verify the scheduling parameter of its container" do
+  test "AlwaysUsePreemptibleInstances makes child containers preemptible" do
+    Rails.configuration.Containers.AlwaysUsePreemptibleInstances = true
     common_attrs = {cwd: "test",
                     priority: 1,
                     command: ["echo", "hello"],
@@ -1086,7 +1167,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
                     state: ContainerRequest::Committed,
                     mounts: {"test" => {"kind" => "json"}}}
     set_user_from_auth :active
-    Rails.configuration.Containers.UsePreemptibleInstances = true
+    configure_preemptible_instance_type
 
     cr = with_container_auth(Container.find_by_uuid 'zzzzz-dz642-runningcontainr') do
       create_minimal_req!(common_attrs)
@@ -1102,6 +1183,32 @@ 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" => 0, "kind" => "tmp"}}}, {mounts: {"/out" => {"kind" => "tmp"}}}],
+   ['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"}],
@@ -1112,12 +1219,18 @@ class ContainerRequestTest < ActiveSupport::TestCase
    ['Final', false, {container_count: 2}],
    ['Final', true, {name: "foobar"}],
    ['Final', true, {name: "foobar", description: "baz"}],
-  ].each do |state, permitted, updates|
+  ].each do |state, permitted, updates, create_attrs|
     test "state=#{state} can#{'not' if !permitted} update #{updates.inspect}" do
       act_as_user users(:active) do
-        cr = create_minimal_req!(priority: 1,
-                                 state: "Committed",
-                                 container_count_max: 1)
+        attrs = {
+          priority: 1,
+          state: "Committed",
+          container_count_max: 1
+        }
+        if !create_attrs.nil?
+          attrs.merge!(create_attrs)
+        end
+        cr = create_minimal_req!(attrs)
         case state
         when 'Committed'
           # already done
@@ -1296,4 +1409,176 @@ class ContainerRequestTest < ActiveSupport::TestCase
       cr.save!
     end
   end
+
+  test "default output_storage_classes" do
+    saved = Rails.configuration.DefaultStorageClasses
+    Rails.configuration.DefaultStorageClasses = ["foo"]
+    begin
+      act_as_user users(:active) do
+        cr = create_minimal_req!(priority: 1,
+                                 state: ContainerRequest::Committed,
+                                 output_name: 'foo')
+        run_container(cr)
+        cr.reload
+        output = Collection.find_by_uuid(cr.output_uuid)
+        assert_equal ["foo"], output.storage_classes_desired
+      end
+    ensure
+      Rails.configuration.DefaultStorageClasses = saved
+    end
+  end
+
+  test "setting output_storage_classes" do
+    act_as_user users(:active) do
+      cr = create_minimal_req!(priority: 1,
+                               state: ContainerRequest::Committed,
+                               output_name: 'foo',
+                               output_storage_classes: ["foo_storage_class", "bar_storage_class"])
+      run_container(cr)
+      cr.reload
+      output = Collection.find_by_uuid(cr.output_uuid)
+      assert_equal ["foo_storage_class", "bar_storage_class"], output.storage_classes_desired
+      log = Collection.find_by_uuid(cr.log_uuid)
+      assert_equal ["foo_storage_class", "bar_storage_class"], log.storage_classes_desired
+    end
+  end
+
+  test "reusing container with different container_request.output_storage_classes" do
+    common_attrs = {cwd: "test",
+                    priority: 1,
+                    command: ["echo", "hello"],
+                    output_path: "test",
+                    runtime_constraints: {"vcpus" => 4,
+                                          "ram" => 12000000000},
+                    mounts: {"test" => {"kind" => "json"}},
+                    environment: {"var" => "value1"},
+                    output_storage_classes: ["foo_storage_class"]}
+    set_user_from_auth :active
+    cr1 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Committed}))
+    cont1 = run_container(cr1)
+    cr1.reload
+
+    output1 = Collection.find_by_uuid(cr1.output_uuid)
+
+    # Testing with use_existing default value
+    cr2 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Uncommitted,
+                                                  output_storage_classes: ["bar_storage_class"]}))
+
+    assert_not_nil cr1.container_uuid
+    assert_nil cr2.container_uuid
+
+    # Update cr2 to commited state, check for reuse, then run it
+    cr2.update_attributes!({state: ContainerRequest::Committed})
+    assert_equal cr1.container_uuid, cr2.container_uuid
+
+    cr2.reload
+    output2 = Collection.find_by_uuid(cr2.output_uuid)
+
+    # the original CR output has the original storage class,
+    # but the second CR output has the new storage class.
+    assert_equal ["foo_storage_class"], cont1.output_storage_classes
+    assert_equal ["foo_storage_class"], output1.storage_classes_desired
+    assert_equal ["bar_storage_class"], output2.storage_classes_desired
+  end
+
+  [
+    [{},               {},           {"type": "output"}],
+    [{"a1": "b1"},     {},           {"type": "output", "a1": "b1"}],
+    [{},               {"a1": "b1"}, {"type": "output", "a1": "b1"}],
+    [{"a1": "b1"},     {"a1": "c1"}, {"type": "output", "a1": "b1"}],
+    [{"a1": "b1"},     {"a2": "c2"}, {"type": "output", "a1": "b1", "a2": "c2"}],
+    [{"type": "blah"}, {},           {"type": "blah"}],
+  ].each do |cr_prop, container_prop, expect_prop|
+    test "setting output_properties #{cr_prop} #{container_prop} on current container" do
+      act_as_user users(:active) do
+        cr = create_minimal_req!(priority: 1,
+                                 state: ContainerRequest::Committed,
+                                 output_name: 'foo',
+                                 output_properties: cr_prop)
+
+        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!(output_properties: container_prop)
+
+          c.update_attributes!(state: Container::Complete,
+                               exit_code: 0,
+                               output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45',
+                               log: logc.portable_data_hash)
+          logc.destroy
+        end
+
+        cr.reload
+        expect_prop["container_request"] = cr.uuid
+        output = Collection.find_by_uuid(cr.output_uuid)
+        assert_equal expect_prop.symbolize_keys, output.properties.symbolize_keys
+      end
+    end
+  end
+
+  test "Cumulative cost includes retried attempts but not reused containers" do
+    set_user_from_auth :active
+    cr = create_minimal_req!(priority: 5, state: "Committed", container_count_max: 3)
+    c = Container.find_by_uuid cr.container_uuid
+    act_as_system_user do
+      c.update_attributes!(state: Container::Locked)
+      c.update_attributes!(state: Container::Running)
+      c.update_attributes!(state: Container::Cancelled, cost: 3)
+    end
+    cr.reload
+    assert_equal 3, cr.cumulative_cost
+
+    c = Container.find_by_uuid cr.container_uuid
+    lock_and_run c
+    c.reload
+    assert_equal 0, c.subrequests_cost
+
+    # cr2 is a child/subrequest
+    cr2 = with_container_auth(c) do
+      create_minimal_req!(priority: 10, state: "Committed", container_count_max: 1, command: ["echo", "foo2"])
+    end
+    assert_equal c.uuid, cr2.requesting_container_uuid
+    c2 = Container.find_by_uuid cr2.container_uuid
+    act_as_system_user do
+      c2.update_attributes!(state: Container::Locked)
+      c2.update_attributes!(state: Container::Running)
+      logc = Collection.new(owner_uuid: system_user_uuid,
+                            manifest_text: ". ef772b2f28e2c8ca84de45466ed19ee9+7815 0:0:arv-mount.txt\n")
+      logc.save!
+      c2.update_attributes!(state: Container::Complete,
+                            exit_code: 0,
+                            output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45',
+                            log: logc.portable_data_hash,
+                            cost: 7)
+    end
+    c.reload
+    assert_equal 7, c.subrequests_cost
+
+    # cr3 is an identical child/subrequest, will reuse c2
+    cr3 = with_container_auth(c) do
+      create_minimal_req!(priority: 10, state: "Committed", container_count_max: 1, command: ["echo", "foo2"])
+    end
+    assert_equal c.uuid, cr3.requesting_container_uuid
+    c3 = Container.find_by_uuid cr3.container_uuid
+    assert_equal c2.uuid, c3.uuid
+    assert_equal Container::Complete, c3.state
+    c.reload
+    assert_equal 7, c.subrequests_cost
+
+    act_as_system_user do
+      c.update_attributes!(state: Container::Complete, exit_code: 0, cost: 9)
+    end
+
+    c.reload
+    assert_equal 7, c.subrequests_cost
+    cr.reload
+    assert_equal 3+7+9, cr.cumulative_cost
+  end
+
 end