Merge branch '13973-child-priority' refs #13973

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

diff --combined services/api/app/models/container_request.rb
index c434ee031773f598b66268bddc434e02aca550a8,4d3dde0a55a4bca4b76071b4ee57298205d1441a..470388a7c7f6786662b4661a454686d9d48e0d15
--- 1/services/api/app/models/container_request.rb
--- 2/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@@ -33,7 -33,6 +33,7 @@@ class ContainerRequest < ArvadosMode
    validates :command, :container_image, :output_path, :cwd, :presence => true
    validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
    validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
 +  validate :validate_datatypes
    validate :validate_scheduling_parameters
    validate :validate_state_change
    validate :check_update_whitelist
@@@ -229,43 -228,6 +229,43 @@@
      end
    end
  
 +  def validate_datatypes
 +    command.each do |c|
 +      if !c.is_a? String
 +        errors.add(:command, "must be an array of strings but has entry #{c.class}")
 +      end
 +    end
 +    environment.each do |k,v|
 +      if !k.is_a?(String) || !v.is_a?(String)
 +        errors.add(:environment, "must be an map of String to String but has entry #{k.class} to #{v.class}")
 +      end
 +    end
 +    [:mounts, :secret_mounts].each do |m|
 +      self[m].each do |k, v|
 +        if !k.is_a?(String) || !v.is_a?(Hash)
 +          errors.add(m, "must be an map of String to Hash but is has entry #{k.class} to #{v.class}")
 +        end
 +        if v["kind"].nil?
 +          errors.add(m, "each item must have a 'kind' field")
 +        end
 +        [[String, ["kind", "portable_data_hash", "uuid", "device_type",
 +                   "path", "commit", "repository_name", "git_url"]],
 +         [Integer, ["capacity"]]].each do |t, fields|
 +          fields.each do |f|
 +            if !v[f].nil? && !v[f].is_a?(t)
 +              errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
 +            end
 +          end
 +        end
 +        ["writable", "exclude_from_output"].each do |f|
 +          if !v[f].nil? && !v[f].is_a?(TrueClass) && !v[f].is_a?(FalseClass)
 +            errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
 +          end
 +        end
 +      end
 +    end
 +  end
 +
    def validate_scheduling_parameters
      if self.state == Committed
        if scheduling_parameters.include? 'partitions' and
@@@ -359,7 -321,13 +359,13 @@@
      c = get_requesting_container()
      if !c.nil?
        self.requesting_container_uuid = c.uuid
-       self.priority = c.priority>0 ? 1 : 0
+       # Determine the priority of container request for the requesting
+       # container.
+       self.priority = ContainerRequest.
+             where('container_uuid=? and priority>0', self.requesting_container_uuid).
+             map do |cr|
+         cr.priority
+       end.max || 0
      end
    end
  

diff --combined services/api/test/unit/container_request_test.rb
index 0a3b9b285e8011e533663033e4349800c128722d,1f6a73c11c781e6a3297adfd1c064ce616b4669d..81b49ff4fcce525b5e7fba88ff0c6f78087e7686
--- 1/services/api/test/unit/container_request_test.rb
--- 2/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@@ -69,7 -69,7 +69,7 @@@ class ContainerRequestTest < ActiveSupp
      cr.container_image = "img3"
      cr.cwd = "/tmp3"
      cr.environment = {"BUP" => "BOP"}
 -    cr.mounts = {"BAR" => "BAZ"}
 +    cr.mounts = {"BAR" => {"kind" => "BAZ"}}
      cr.output_path = "/tmp4"
      cr.priority = 2
      cr.runtime_constraints = {"vcpus" => 4}
@@@ -81,33 -81,29 +81,33 @@@
    end
  
    [
 -    {"vcpus" => 1},
 -    {"vcpus" => 1, "ram" => nil},
 -    {"vcpus" => 0, "ram" => 123},
 -    {"vcpus" => "1", "ram" => "123"}
 -  ].each do |invalid_constraints|
 -    test "Create with #{invalid_constraints}" do
 +    {"runtime_constraints" => {"vcpus" => 1}},
 +    {"runtime_constraints" => {"vcpus" => 1, "ram" => nil}},
 +    {"runtime_constraints" => {"vcpus" => 0, "ram" => 123}},
 +    {"runtime_constraints" => {"vcpus" => "1", "ram" => "123"}},
 +    {"mounts" => {"FOO" => "BAR"}},
 +    {"mounts" => {"FOO" => {}}},
 +    {"mounts" => {"FOO" => {"kind" => "tmp", "capacity" => 42.222}}},
 +    {"command" => ["echo", 55]},
 +    {"environment" => {"FOO" => 55}}
 +  ].each do |value|
 +    test "Create with invalid #{value}" do
        set_user_from_auth :active
        assert_raises(ActiveRecord::RecordInvalid) do
 -        cr = create_minimal_req!(state: "Committed",
 -                                 priority: 1,
 -                                 runtime_constraints: invalid_constraints)
 +        cr = create_minimal_req!({state: "Committed",
 +               priority: 1}.merge(value))
          cr.save!
        end
      end
  
 -    test "Update with #{invalid_constraints}" do
 +    test "Update with invalid #{value}" do
        set_user_from_auth :active
        cr = create_minimal_req!(state: "Uncommitted", priority: 1)
        cr.save!
        assert_raises(ActiveRecord::RecordInvalid) do
          cr = ContainerRequest.find_by_uuid cr.uuid
 -        cr.update_attributes!(state: "Committed",
 -                              runtime_constraints: invalid_constraints)
 +        cr.update_attributes!({state: "Committed",
 +                               priority: 1}.merge(value))
        end
      end
    end
@@@ -379,7 -375,7 +379,7 @@@
    end
  
    [
-     ['running_container_auth', 'zzzzz-dz642-runningcontainr', 1],
+     ['running_container_auth', 'zzzzz-dz642-runningcontainr', 501],
      ['active_no_prefs', nil, 0],
    ].each do |token, expected, expected_priority|
      test "create as #{token} and expect requesting_container_uuid to be #{expected}" do