Arvados

I have some questions about this story:

1. The fact that the uuid is considered advisory when passing both pdh & uuid means that it should be preserved on the resolved mount instead of being deleted? I ask because there're tests checking that it should be deleted.
2. When both pdh & uuid are passed, should we still have to check that they match, right? There's also a test confirming this.
3. Always resolving to PDH (this time giving priority to PDH when both are given) means that this story is only about the API server behavior change, or is something else needed?

In the meantime:

Updates at d8a9500ef - branch 14323-container-collection-mounts
Test run: https://ci.curoverse.com/job/developer-run-tests/970/

• Prioritize pdh over uuid when both provided
• Don't delete uuid key on resolved mounts (as I understand, if they're advisory data, they shouldn't be deleted)
• Update tests

The container request mounts can have UUID, PDH, or both. If both are given, the UUID is ignored (but preserved).

The container mounts ("resolved mounts") never have UUID, always PDH (so the mount.delete 'uuid' line should still be there). Including the UUID would interfere with container reuse.

If a container request's PDH and UUID don't match, we use the PDH anyway. This might be the only actual change here: previously this caused an error, which meant an update race would cause failures for a client that looks up the UUIDs by itself (e.g., to ensure all containers in a workflow use the same PDH for a given UUID) but still wants to preserve the UUIDs as provenance information.

Updates at 467250f94
Test run: https://ci.curoverse.com/job/developer-run-tests/971/

Following above comments, removed the collection lookup by pdh, and only try getting the collection mount by uuid when the pdh is not provided. Updated tests.

Indentation mishap in container_request_test.rb.

Rest LGTM, thanks!

Some workbench integration tests failed -- fix at 50288d61c8654a87053a2b3492672cf7d8d71930
Test run: https://ci.curoverse.com/job/developer-run-tests/974/

Looks like this restores the possibility of a container mount with no UUID and no PDH. This means "start with empty collection" according to the Containers API spec. Seems like we need a test for this in the API suite, rather than relying on WB integration tests.

With that, LGTM.

Added test at 2480ed0b8, testing one more time before merging: https://ci.curoverse.com/job/developer-run-tests/975/