Feature #8465
closed
[Crunch2] Support stdin/stderr redirection
Added by Peter Amstutz almost 9 years ago.
Updated almost 8 years ago.
Assigned To:
Radhika Chippada
Estimated time:
(Total: 0.00 h)
Release relationship:
Auto
Description
For stdin redirection:
- After arv-mount is started
- Look for "stdin" in mounts
- Open file with stdin content (must support kind: collection, ideally should support kind: json as well)
- Attach stdin to docker container, I think this is
ContainerAttachOptions{Stdin: true}
- Copy input file to docker container stdin stream, I think this is a goroutine with
io.Copy(infile, response.Conn)
For stderr redirection:
Copy/refactor code for existing stdout redirection.
- Subject changed from [Crunch2[ Support stdin redirection to [Crunch2] Support stdin redirection
- Assigned To set to Radhika Chippada
- Target version set to Arvados Future Sprints
- Description updated (diff)
- Assigned To deleted (
Radhika Chippada)
- Target version changed from Arvados Future Sprints to 2017-04-12 sprint
- Story points set to 1.0
- Assigned To set to Radhika Chippada
- Subject changed from [Crunch2] Support stdin redirection to [Crunch2] Support stdin/stderr redirection
- Description updated (diff)
- Status changed from New to In Progress
Branch 8465-stdin-redirection @ c3ae1e2
- Added stdin redirection for collection and json mount points
- Added tests
- Currently, the tests provide stdin but do not do anything with it. It would be desirable that the command does "echo stdin-content" or something, but I could not (yet) figure out how to do this.
- Status changed from In Progress to New
crunchrun.go L690:
stdinRdr, err = runner.Kc.CollectionFileReader(map[string]interface{}{"manifest_text": stdinColl.ManifestText}, stdinMnt.Path)
Can write this as:
stdinRdr, err = runner.Kc.ManifestFileReader(manifest.Manifest{Text: stdinColl.ManifestText}, stdinMnt.Path)
It looks like it waits for the entire contents to copied to stdin. That's not right, because AttachStreams() is called from CreateContainer(), but before StartContainer(), which means the container is not running yet (so there is nothing ready to accept the stdin stream.) You need to start the goroutine and then return from AttachStreams (so the goroutine continues running).
If there is an error from io.Copy(), it should be log the error using runner.CrunchLog and call runner.stop() to cancel the container.
The same applies to json content on stdin.
Can write this as: stdinRdr, err = runner.Kc.ManifestFileReader(manifest.Manifest{Text: stdinColl.ManifestText}, stdinMnt.Path)
Done (and removed unused CollectionFileReader references)
It looks like it waits for the entire contents to copied to stdin. That's not right, because AttachStreams() is called from CreateContainer(), but before StartContainer(), which means the container is not running yet (so there is nothing ready to accept the stdin stream.) You need to start the goroutine and then return from AttachStreams (so the goroutine continues running).
Updated accordingly
If there is an error from io.Copy(), it should be log the error using runner.CrunchLog and call runner.stop() to cancel the container.
Done
The same applies to json content on stdin
Done
New branch 8465-stderr-redirection implements stderr redirection. This branch is off of 8465-stdin-redirection. Thanks.
- Status changed from New to In Progress
Branch 8465-cwl-containers-stdin-stderr LGTM
- Status changed from In Progress to Resolved
- % Done changed from 75 to 100
Applied in changeset arvados|commit:a734789218122d8ab0d8f766bac4d69c04db91bf.
Also available in: Atom
PDF
Updated by 
Updated by Radhika Chippada 
Updated by Tom Morris 
Updated by