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.
Updated by Peter Amstutz almost 10 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 10 years ago
Should also include satisfying defaults such as RAM and CPUs.
Updated by Tom Clegg almost 10 years ago
- Category set to Crunch
- Assigned To set to Tom Clegg
Updated by Tom Clegg almost 10 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 Tom Clegg almost 10 years ago
- Target version set to 2016-07-06 sprint
Updated by Radhika Chippada over 9 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 over 9 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 over 9 years ago
- Status changed from In Progress to Resolved
- % Done changed from 75 to 100
Applied in changeset arvados|commit:01ea0e9faa0b29ef747699f7f4b728d4e888ef83.
Actions