> On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 207
> > <https://reviews.apache.org/r/28920/diff/7/?file=808124#file808124line207>
> >
> >     What are the implications of this?  Does it mean a docker image must be 
> > pre-loaded on the host for it to be used?  Can we cope with the user 
> > specifying a bad path?  If not, what's the fallout - TASK_FAILED?
> 
> Steve Niemitz wrote:
>     The Volume options are for mounting paths on the host into the docker 
> container, and correspond to the -v flag of docker run 
> (https://docs.docker.com/userguide/dockervolumes/#mount-a-host-directory-as-a-data-volume).
>   If the path is bad, it will fail but continue to run the container.  This 
> is goverened by mesos, and I actually have some plans to enhance their docker 
> integration.
> 
> Bill Farner wrote:
>     Awesome, please include that link here and in the .md.
>     
>     Question remains about whether the image must be pre-loaded on the 
> machine.

Will do.  I'll document it as well but the image doesn't need to be on the 
machine, Mesos will pull it if it doesn't exist.


> On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
> > docs/deploying-aurora-scheduler.md, line 155
> > <https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155>
> >
> >     From reading the code, it appears this additional path is needed for 
> > them both to be available in the container.  If so, would it be easier to 
> > just accept an arbitrary number of additional assets to copy into the 
> > sandbox?  I would find that more generalized, and easier to understand.
> >     
> >     If you go with the above, i _think_ you can also safely nuke the extra 
> > args plumbing.
> 
> Steve Niemitz wrote:
>     I think we'll still need extra args (its really useful right now to pass 
> ZK config in), but I like the idea of an arbitrary set of resources.  The 
> only trouble here would be figuring out which is the one to run and getting 
> the symlinks right.  Let's talk about this more.
> 
> Bill Farner wrote:
>     Wouldn't the extra args just be solved with the shim script?  I tell the 
> scheduler to copy `my_executor.sh` and `executor.pex` into the container, and 
> `my_executor.sh` contains the extra args.  I like this as a generalized 
> solution to augmenting default executor behavior, since it avoids feature 
> creep on our side just to save people the shim script.

Correct, it would.  My goal with this (which I'm happy to discuss more) was to 
make the wrapper script unnecessary in normal cases.  For us we just need to 
pass in the ZK config for the announcer, and I feel like this is the typical 
case.  Let me simmer on this one for a little bit.


> On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 109
> > <https://reviews.apache.org/r/28920/diff/7/?file=808132#file808132line109>
> >
> >     It's good form for this type of sanitization to happen here, but at 
> > minimum the same sanitization must be done in `ConfigurationManager` to 
> > give the user a good message and avoid accepting the bad config.
> 
> Steve Niemitz wrote:
>     I do a similar check in SchedulerMain.java, ~line 211.  Should I move the 
> check into ConfigurationManager?  ExecutorSettings.ctor checks as well.
> 
> Bill Farner wrote:
>     Doh, i left this comment in the wrong place.  This should have been a 
> request to sanitize the incoming thrift fields.

Ah that makes more sense!  I'll do so.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28920/#review67226
-----------------------------------------------------------


On Jan. 8, 2015, 8:35 p.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-633
>     https://issues.apache.org/jira/browse/AURORA-633
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This change adds support for launching docker containers through aurora.  
> These changes are based off of the discussion in 
> https://issues.apache.org/jira/browse/AURORA-633
> 
> As of now, a special thermos_executor.sh script is needed to launch the 
> executor inside docker containers.  A sample aurora file is in 
> examples/jobs/docker.
> 
> In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
> the example upstart config in examples/vagrant/upstart has been updated to 
> reflect this.
> 
> More information is in subsequent review request comments.
> 
> 
> Diffs
> -----
> 
>   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 08ba1cdf88b712de22c26c04443079282db59ef9 
>   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
>   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
>   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
>   examples/vagrant/aurorabuild.sh b7ea41719a8f41bb23d0254e732926d89399c77c 
>   examples/vagrant/upstart/mesos-slave.conf 
> 512ce7ecf34042ed68dda55efb2dd0415f8469db 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
>   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
> 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> d885b224ec5a1d529347d84e03ba98ab6734a126 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
>   src/main/python/apache/aurora/config/thrift.py 
> ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 636b23d30a897b557eb8c3f8733c90b23cb807ef 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
>   src/main/python/apache/aurora/executor/common/sandbox.py 
> f47a32b3fefb4a89940b1ddc473b8316ac00df12 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 5e4bd65537d186459003c0b9434f1b769e04f448 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/runner.py 
> 8aac6b50c66080abbb5308b367e9f74c487f42e3 
>   src/main/resources/scheduler/assets/configSummary.html 
> 28878908b0c9381e366b71a3135dfc28c542a890 
>   src/main/resources/scheduler/assets/js/services.js 
> b744f375411e09b7f776e4a05ee5961227143439 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 5e54364a49a208bd5f19b9649633dc8feca591e9 
>   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
> 876e173ccbac04e4a06a245648c7c6af15eaaa92 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 503e62f4cac872b14f6985b5bccc3e4dfcf81789 
> 
> Diff: https://reviews.apache.org/r/28920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Reply via email to