Story #9132
closed[Crunch2] crunch-run uses official docker client library
100%
Description
Current crunch-run is using curoverse/dockerclient, but this is not actively maintained and lacks support for attaching stdin. The supported Go API for Docker is Docker's own client code https://github.com/docker/docker/tree/master/client
This should be a straight-across port to equivalent functionality.
Updated by Tom Morris over 8 years ago
- Target version set to Arvados Future Sprints
Updated by Peter Amstutz almost 8 years ago
- Subject changed from [Crunch2] crunch-run uses docker/engine-api instead of dockerclient to [Crunch2] crunch-run uses official docker client library
Updated by Tom Morris almost 8 years ago
- Target version changed from Arvados Future Sprints to 2017-03-15 sprint
Updated by Peter Amstutz almost 8 years ago
- Status changed from New to In Progress
Updated by Peter Amstutz almost 8 years ago
- Target version changed from 2017-03-15 sprint to 2017-03-29 sprint
Updated by Peter Amstutz almost 8 years ago
- Assigned To changed from Peter Amstutz to Radhika Chippada
Updated by Peter Amstutz almost 8 years ago
Please take over 9132-dockerclient
- Replace API calls to equivalent ones, see https://godoc.org/github.com/docker/docker/client
- Update stubs used by tests
- Once it compiles & tests pass, run integration tests using arvbox and "arvados-cwl-runner --api=containers"
Updated by Radhika Chippada almost 8 years ago
Branch 9132-dockerclient @ 374dff075f0d2961b3843d952ccca9f55d972e9a
- Uses new docker client API.
- What ContainerStartOptions should we use?
- The ContainerWait method returns an int64 and hence I replaced the waitDocker chan as such.
- The test "TestStopOnSignal" is failing sometimes when the whole crunch-run test set is executed with the error “cannot allocate memory” locally, but it appears that it is an issue with development env and passing consistently in jenkins
Thanks.
Updated by Tom Clegg almost 8 years ago
Radhika Chippada wrote:
- What ContainerStartOptions should we use?
Empty LGTM. The options seem to be for checkpoint suspend/resume -- sounds interesting, but we're not using it.
- The ContainerWait method returns an int64 and hence I replaced the waitDocker chan as such.
nit: could change the "finish" field and the "exitCode" arguments too, then the int64() cast wouldn't be needed.
- The test "TestStopOnSignal" is failing sometimes when the whole crunch-run test set is executed with the error “cannot allocate memory” locally, but it appears that it is an issue with development env and passing consistently in jenkins
Tried run-tests.sh --repeat 20, no problems.
LGTM, thanks!
Updated by Peter Amstutz almost 8 years ago
I'm running stuff and getting an exit code of "2" when it should be "0". I think maybe the return code from ContainerWait is not being interpreted properly?
Updated by Radhika Chippada almost 8 years ago
I think maybe the return code from ContainerWait is not being interpreted properly
We are just returning what is returned by ContainerWait and so we don't have any interpretation happening.
However, according to https://docs.docker.com/engine/reference/run/#exit-status , which takes me to http://tldp.org/LDP/abs/html/exitcodes.html , it could be that we are not configuring the client correctly.
This stackflow page led me to the above links: http://stackoverflow.com/questions/31297616/what-is-the-authoritative-list-of-docker-run-exit-codes
Updated by Peter Amstutz almost 8 years ago
Yea, that might be the legitimate exit code. But the CWL test should pass, and it isn't, so it needs further investigation.
Updated by Radhika Chippada almost 8 years ago
- Assigned To changed from Radhika Chippada to Peter Amstutz
Assigning to Peter to debug / fix any issues with docket client invocations. Thanks.
Updated by Peter Amstutz almost 8 years ago
- Target version changed from 2017-03-29 sprint to 2017-04-12 sprint
Updated by Peter Amstutz almost 8 years ago
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:b9236fbe81426446e1b541a45e219bbe513fe8d0.