Project

General

Profile

Actions

Feature #8470

closed

[Crunch2] [API] Use exact values for runtime_constraints and mounts in Container records

Added by Peter Amstutz over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Crunch
Target version:
Story points:
1.0
Release:
Release relationship:
Auto

Description

Currently, the API satisfies a Container Request by copying its attributes into a new Container.

However, a Container record is meant to specify a container unambiguously, while a Container Request describes a set of acceptable Containers (e.g., "between 1 and 2 GB of RAM") and uses identifiers like UUIDs and git branch names, which resolve to different values at different times.

New behavior:
  • If cr.container_image is not already a collection PDH, resolve it (as a docker tag or a collection UUID) to a PDH. Use the resulting PDH as the container_image for the new Container.
  • If any entries in cr.mounts specify collection UUIDs, look them up and specify them as PDH instead in the new Container.
  • If any entries in runtime_constraints specify ranges (e.g., "vcpus":[2,null] signifying "at least two cores"), use the minimum (2 in this case) as the value for the corresponding key in the new Container's runtime_constraints.

Subtasks 4 (0 open4 closed)

Task #9477: Use numbers for runtime constraintsResolvedTom Clegg06/23/2016Actions
Task #9478: Resolve collections to PDHResolvedTom Clegg06/23/2016Actions
Task #9479: Resolve docker image tag to PDHResolvedTom Clegg06/24/2016Actions
Task #9480: Review 8470-resolve-container-reqResolvedRadhika Chippada06/27/2016Actions

Related issues

Related to Arvados - Idea #9273: [Crunch2] Do not satisfy container requests that use non-content-address names for docker images, git commits, or collectionsClosedActions
Actions #1

Updated by Peter Amstutz over 8 years ago

  • Subject changed from [Crunch2] API resolves Docker, API, collection uuids to content addresses in container request to [Crunch2] API resolves Docker, git, collection uuids to content addresses in container request
Actions #2

Updated by Peter Amstutz over 8 years ago

Should also include satisfying defaults such as RAM and CPUs.

Actions #3

Updated by Tom Clegg over 8 years ago

  • Category set to Crunch
  • Assigned To set to Tom Clegg
Actions #4

Updated by Tom Clegg over 8 years ago

  • Subject changed from [Crunch2] API resolves Docker, git, collection uuids to content addresses in container request to [Crunch2] [API] Use exact values for runtime_constraints and mounts in Container records
  • Description updated (diff)
Actions #5

Updated by Tom Clegg over 8 years ago

  • Target version set to 2016-07-06 sprint
Actions #6

Updated by Tom Clegg over 8 years ago

  • Story points set to 1.0
Actions #7

Updated by Tom Clegg over 8 years ago

  • Status changed from New to In Progress
Actions #8

Updated by Tom Clegg over 8 years ago

8470-resolve-container-req @ be4ca15

Actions #9

Updated by Radhika Chippada over 8 years ago

container_request.rb

  • In resolve method
    • the comment “TODO: resolve container_image to a content address.” seems to have been addressed in container_image_for_container method?
    • If it is not too much to ask, in “Container.create!(command: self.command,” can you place all self.X items first and then the derived ones to improve readability of code?
  • In mounts_for_container, line 139, can we say “mount #{uuid} not found” instead of “collection #{uuid} …”? Also, “uuid #{uuid} has portable_data_hash …”, can you say “mount uuid #{uuid} has …”?

container_request_test.rb

  • In ‘test "Container request max priority" do’ , please add a comment explaining why cr2 priority is expected.
  • In ‘test “Container makes container request, then is cancelled" do’ :
    • Add an assertion to check priority on cr after line 223 (after cancel)
    • I think you can use create_minimal_req! for the first cr creation
    • Can you please add a comment in this or in test “container cancelled finalizes request” or in code listing canceling what cancels what. I think a comment listing the relationships between Cs and CRs and Requesting_Cs and Container_uuids would be quite helpful.
  • “resolve runtime constraint range #{mounts} to values” => mounts is not from runtime constraints, correct? should the name of this test be something like “resolve mounts to values …”?
Actions #10

Updated by Tom Clegg over 8 years ago

Radhika Chippada wrote:

  • the comment “TODO: resolve container_image to a content address.” seems to have been addressed in container_image_for_container method?

indeed. removed.

  • If it is not too much to ask, in “Container.create!(command: self.command,” can you place all self.X items first and then the derived ones to improve readability of code?

done

  • In mounts_for_container, line 139, can we say “mount #{uuid} not found” instead of “collection #{uuid} …”? Also, “uuid #{uuid} has portable_data_hash …”, can you say “mount uuid #{uuid} has …”?

improved error messages

  • In ‘test "Container request max priority" do’ , please add a comment explaining why cr2 priority is expected.

done

  • In ‘test “Container makes container request, then is cancelled" do’ :
    • Add an assertion to check priority on cr after line 223 (after cancel)
    • I think you can use create_minimal_req! for the first cr creation
    • Can you please add a comment in this or in test “container cancelled finalizes request” or in code listing canceling what cancels what. I think a comment listing the relationships between Cs and CRs and Requesting_Cs and Container_uuids would be quite helpful.

fixed up some comments here

  • “resolve runtime constraint range #{mounts} to values” => mounts is not from runtime constraints, correct? should the name of this test be something like “resolve mounts to values …”?

whoops, yes, fixed... thanks

Actions #11

Updated by Tom Clegg over 8 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 75 to 100

Applied in changeset arvados|commit:01ea0e9faa0b29ef747699f7f4b728d4e888ef83.

Actions

Also available in: Atom PDF