Story #9132

[Crunch2] crunch-run uses official docker client library

Added by Peter Amstutz over 1 year ago. Updated 6 months ago.

Status:ResolvedStart date:03/08/2017
Priority:NormalDue date:
Assignee:Peter Amstutz% Done:

100%

Category:-
Target version:2017-04-12 sprint
Story points1.0Remaining (hours)0.00 hour
Velocity based estimate-
ReleaseCrunch v2

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.


Subtasks

Task #11192: Review 9132-dockerclientResolvedRadhika Chippada

Task #11367: Need to pass CWL testsResolvedPeter Amstutz

Task #11224: Update API usageResolvedPeter Amstutz


Related issues

Blocks Arvados - Feature #8465: [Crunch2] Support stdin/stderr redirection Resolved 04/07/2017

Associated revisions

Revision b9236fbe
Added by Peter Amstutz 6 months ago

Merge branch '9132-dockerclient' closes #9132

History

#1 Updated by Peter Amstutz over 1 year ago

  • Description updated (diff)

#2 Updated by Tom Morris about 1 year ago

  • Target version set to Arvados Future Sprints

#3 Updated by Peter Amstutz 7 months ago

  • Description updated (diff)

#4 Updated by Peter Amstutz 7 months ago

  • Subject changed from [Crunch2] crunch-run uses docker/engine-api instead of dockerclient to [Crunch2] crunch-run uses official docker client library

#5 Updated by Peter Amstutz 7 months ago

  • Description updated (diff)

#6 Updated by Tom Morris 7 months ago

  • Target version changed from Arvados Future Sprints to 2017-03-15 sprint

#7 Updated by Peter Amstutz 7 months ago

  • Assignee set to Peter Amstutz

#8 Updated by Peter Amstutz 7 months ago

  • Status changed from New to In Progress

#9 Updated by Peter Amstutz 6 months ago

  • Target version changed from 2017-03-15 sprint to 2017-03-29 sprint

#10 Updated by Peter Amstutz 6 months ago

  • Assignee changed from Peter Amstutz to Radhika Chippada

#11 Updated by Peter Amstutz 6 months ago

Please take over 9132-dockerclient

#12 Updated by Radhika Chippada 6 months 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.

#13 Updated by Tom Clegg 6 months 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!

#14 Updated by Peter Amstutz 6 months 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?

#15 Updated by Radhika Chippada 6 months 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

#16 Updated by Peter Amstutz 6 months ago

Yea, that might be the legitimate exit code. But the CWL test should pass, and it isn't, so it needs further investigation.

#17 Updated by Radhika Chippada 6 months ago

  • Assignee changed from Radhika Chippada to Peter Amstutz

Assigning to Peter to debug / fix any issues with docket client invocations. Thanks.

#18 Updated by Peter Amstutz 6 months ago

  • Target version changed from 2017-03-29 sprint to 2017-04-12 sprint

#19 Updated by Peter Amstutz 6 months ago

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

Applied in changeset arvados|commit:b9236fbe81426446e1b541a45e219bbe513fe8d0.

Also available in: Atom PDF