Feature #8465
closed
[Crunch2] Support stdin/stderr redirection
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.
Related issues
Updated by Brett Smith about 8 years ago
- Subject changed from [Crunch2[ Support stdin redirection to [Crunch2] Support stdin redirection
Updated by Radhika Chippada almost 8 years ago
- Assigned To set to Radhika Chippada
Updated by Tom Morris about 7 years ago
- Target version set to Arvados Future Sprints
Updated by Peter Amstutz about 7 years ago
- Description updated (diff)
- Assigned To deleted (
Radhika Chippada)
Updated by Tom Morris about 7 years ago
- Target version changed from Arvados Future Sprints to 2017-04-12 sprint
- Story points set to 1.0
Updated by Radhika Chippada about 7 years ago
- Assigned To set to Radhika Chippada
Updated by Peter Amstutz about 7 years ago
- Subject changed from [Crunch2] Support stdin redirection to [Crunch2] Support stdin/stderr redirection
- Description updated (diff)
Updated by Radhika Chippada about 7 years ago
- Status changed from New to In Progress
Updated by Radhika Chippada about 7 years ago
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.
Updated by Peter Amstutz about 7 years ago
- 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.
Updated by Radhika Chippada about 7 years ago
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
Updated by Radhika Chippada about 7 years ago
New branch 8465-stderr-redirection implements stderr redirection. This branch is off of 8465-stdin-redirection. Thanks.
Updated by Radhika Chippada about 7 years ago
- Status changed from New to In Progress
Updated by Peter Amstutz about 7 years ago
8465-stderr-redirection @ c1ea9a2fdccc5e24554f8cf28296cbc0764c8bbc LGTM, please merge
Updated by Radhika Chippada about 7 years ago
Branch 8465-cwl-containers-stdin-stderr LGTM
Updated by Peter Amstutz about 7 years ago
- Status changed from In Progress to Resolved
- % Done changed from 75 to 100
Applied in changeset arvados|commit:a734789218122d8ab0d8f766bac4d69c04db91bf.