Actions
Feature #8470
closed
[Crunch2] [API] Use exact values for runtime_constraints and mounts in Container records
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_imageis 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.mountsspecify 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 (2in this case) as the value for the corresponding key in the new Container's runtime_constraints.
Related issues
Updated by Peter Amstutz almost 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
Updated by Peter Amstutz almost 8 years ago
Should also include satisfying defaults such as RAM and CPUs.
Updated by Tom Clegg almost 8 years ago
- Category set to Crunch
- Assigned To set to Tom Clegg
Updated by Tom Clegg almost 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)
Updated by Radhika Chippada almost 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 …”?
Updated by Tom Clegg almost 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
Updated by Tom Clegg almost 8 years ago
- Status changed from In Progress to Resolved
- % Done changed from 75 to 100
Applied in changeset arvados|commit:01ea0e9faa0b29ef747699f7f4b728d4e888ef83.
Actions