Merge branch '13996-new-api-config' refs #13996

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --combined doc/_config.yml
index 510fcc726a6274159d01e2f584cbddba45c83429,57e5ec01bdf0e3a982f76df1c72d586812e8b7e8..c7f9bd19788448ef97af679ba917ed2cba530d27
--- 1/doc/_config.yml
--- 2/doc/_config.yml
+++ b/doc/_config.yml
@@@ -154,8 -154,11 +154,11 @@@ navbar
    admin:
      - Topics:
        - admin/index.html.textile.liquid
+     - Configuration:
+       - admin/config.html.textile.liquid
      - Upgrading and migrations:
        - admin/upgrading.html.textile.liquid
+       - admin/config-migration.html.textile.liquid
        - install/migrate-docker19.html.textile.liquid
        - admin/upgrade-crunch2.html.textile.liquid
      - Users and Groups:
@@@ -174,8 -177,6 +177,8 @@@
      - Other:
        - admin/collection-versioning.html.textile.liquid
        - admin/federation.html.textile.liquid
 +      - admin/controlling-container-reuse.html.textile.liquid
 +      - admin/logs-table-management.html.textile.liquid
    installguide:
      - Overview:
        - install/index.html.textile.liquid

diff --combined services/api/app/models/container.rb
index d251f88274a5abca109b1f52c6ede78043016a17,95691687e6d77c2d51c1287136ad3240869e81de..45cd13bbcddbc762f3d828a30454505082fce528
--- 1/services/api/app/models/container.rb
--- 2/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@@ -89,8 -89,7 +89,8 @@@ class Container < ArvadosMode
      nil => [Queued],
      Queued => [Locked, Cancelled],
      Locked => [Queued, Running, Cancelled],
 -    Running => [Complete, Cancelled]
 +    Running => [Complete, Cancelled],
 +    Complete => [Cancelled]
    }
  
    def self.limit_index_columns_read
@@@ -206,7 -205,7 +206,7 @@@
      rc = {}
      defaults = {
        'keep_cache_ram' =>
-       Rails.configuration.container_default_keep_cache_ram,
+       Rails.configuration.Containers.DefaultKeepCacheRAM,
      }
      defaults.merge(runtime_constraints).each do |k, v|
        if v.is_a? Array
@@@ -369,7 -368,7 +369,7 @@@
      transaction do
        reload(lock: 'FOR UPDATE')
        check_unlock_fail
-       if self.lock_count < Rails.configuration.max_container_dispatch_attempts
+       if self.lock_count < Rails.configuration.Containers.MaxDispatchAttempts
          update_attributes!(state: Queued)
        else
          update_attributes!(state: Cancelled,
@@@ -498,7 -497,7 +498,7 @@@
        return false
      end
  
 -    if self.state == Running &&
 +    if self.state_was == Running &&
         !current_api_client_authorization.nil? &&
         (current_api_client_authorization.uuid == self.auth_uuid ||
          current_api_client_authorization.token == self.runtime_token)
@@@ -506,8 -505,6 +506,8 @@@
        # change priority or log.
        permitted.push *final_attrs
        permitted = permitted - [:log, :priority]
 +    elsif !current_user.andand.is_admin
 +      raise PermissionDeniedError
      elsif self.locked_by_uuid && self.locked_by_uuid != current_api_client_authorization.andand.uuid
        # When locked, progress fields cannot be updated by the wrong
        # dispatcher, even though it has admin privileges.

diff --combined services/api/test/unit/container_test.rb
index 320774189fc60ad1b63362c427e71c4ee2cdffb5,11429affab1927fa8b7cec798f7a0510452a7856..88fd5feb6ad27c3c55dd5531c0a2566422239f41
--- 1/services/api/test/unit/container_test.rb
--- 2/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@@ -184,7 -184,7 +184,7 @@@ class ContainerTest < ActiveSupport::Te
      assert_equal c1.runtime_status, {}
  
      assert_equal Container::Queued, c1.state
 -    assert_raises ActiveRecord::RecordInvalid do
 +    assert_raises ArvadosModel::PermissionDeniedError do
        c1.update_attributes! runtime_status: {'error' => 'Oops!'}
      end
  
@@@ -241,7 -241,7 +241,7 @@@
    end
  
    test "find_reusable method should select higher priority queued container" do
-         Rails.configuration.log_reuse_decisions = true
+         Rails.configuration.Containers.LogReuseDecisions = true
      set_user_from_auth :active
      common_attrs = REUSABLE_COMMON_ATTRS.merge({environment:{"var" => "queued"}})
      c_low_priority, _ = minimal_new(common_attrs.merge({use_existing:false, priority:1}))
@@@ -511,7 -511,7 +511,7 @@@
  
    test "find_reusable with logging enabled" do
      set_user_from_auth :active
-     Rails.configuration.log_reuse_decisions = true
+     Rails.configuration.Containers.LogReuseDecisions = true
      Rails.logger.expects(:info).at_least(3)
      Container.find_reusable(REUSABLE_COMMON_ATTRS)
    end
@@@ -666,7 -666,7 +666,7 @@@
    end
  
    test "Exceed maximum lock-unlock cycles" do
-     Rails.configuration.max_container_dispatch_attempts = 3
+     Rails.configuration.Containers.MaxDispatchAttempts = 3
  
      set_user_from_auth :active
      c, cr = minimal_new
@@@ -777,51 -777,6 +777,51 @@@
      end
    end
  
 +  [
 +    [Container::Queued, {state: Container::Locked}],
 +    [Container::Queued, {state: Container::Running}],
 +    [Container::Queued, {state: Container::Complete}],
 +    [Container::Queued, {state: Container::Cancelled}],
 +    [Container::Queued, {priority: 123456789}],
 +    [Container::Queued, {runtime_status: {'error' => 'oops'}}],
 +    [Container::Queued, {cwd: '/'}],
 +    [Container::Locked, {state: Container::Running}],
 +    [Container::Locked, {state: Container::Queued}],
 +    [Container::Locked, {priority: 123456789}],
 +    [Container::Locked, {runtime_status: {'error' => 'oops'}}],
 +    [Container::Locked, {cwd: '/'}],
 +    [Container::Running, {state: Container::Complete}],
 +    [Container::Running, {state: Container::Cancelled}],
 +    [Container::Running, {priority: 123456789}],
 +    [Container::Running, {runtime_status: {'error' => 'oops'}}],
 +    [Container::Running, {cwd: '/'}],
 +    [Container::Complete, {state: Container::Cancelled}],
 +    [Container::Complete, {priority: 123456789}],
 +    [Container::Complete, {runtime_status: {'error' => 'oops'}}],
 +    [Container::Complete, {cwd: '/'}],
 +    [Container::Cancelled, {cwd: '/'}],
 +  ].each do |start_state, updates|
 +    test "Container update #{updates.inspect} when #{start_state} forbidden for non-admin" do
 +      set_user_from_auth :active
 +      c, _ = minimal_new
 +      if start_state != Container::Queued
 +        set_user_from_auth :dispatch1
 +        c.lock
 +        if start_state != Container::Locked
 +          c.update_attributes! state: Container::Running
 +          if start_state != Container::Running
 +            c.update_attributes! state: start_state
 +          end
 +        end
 +      end
 +      assert_equal c.state, start_state
 +      set_user_from_auth :active
 +      assert_raises(ArvadosModel::PermissionDeniedError) do
 +        c.update_attributes! updates
 +      end
 +    end
 +  end
 +
    test "Container only set exit code on complete" do
      set_user_from_auth :active
      c, _ = minimal_new
@@@ -944,9 -899,7 +944,9 @@@
      c.update_attributes! state: Container::Running
  
      set_user_from_auth :running_to_be_deleted_container_auth
 -    refute c.update_attributes(output: collections(:foo_file).portable_data_hash)
 +    assert_raises(ArvadosModel::PermissionDeniedError) do
 +      c.update_attributes(output: collections(:foo_file).portable_data_hash)
 +    end
    end
  
    test "can set trashed output on running container" do