> On Jan. 14, 2015, 1:03 a.m., Bill Farner wrote:
> > examples/vagrant/upstart/mesos-slave.conf, line 32
> > <https://reviews.apache.org/r/28920/diff/13/?file=818522#file818522line32>
> >
> >     Is this afflicted by MESOS-2215?  I'm worried (read: certain) people 
> > will copy/paste the example here, and likely be set up for failure.  I 
> > don't have an answer, curious what your thoughts are.

If checkpointing is enabled (which it isnt by default in the scheduler), it is 
affected.  Hopefully the mesos issue can get patched soon.  Unless someone 
explicitly enabled checkpointing in the scheduler they're fine.


> On Jan. 14, 2015, 1:03 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 95
> > <https://reviews.apache.org/r/28920/diff/13/?file=818523#file818523line95>
> >
> >     The Args system handles multi-valued args, comma-separated.  Change to 
> > `Arg<List<String>>` and this will Just Work.

Fancy


> On Jan. 14, 2015, 1:03 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java, line 100
> > <https://reviews.apache.org/r/28920/diff/13/?file=818525#file818525line100>
> >
> >     shell=true is default, is there a reason you want to explicitly set it? 
> >  I'm not against it, just want to be on the same page.

No reason in particular other than to make it explicit, especially since false 
tends to be the default for things.


> On Jan. 14, 2015, 1:03 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java,
> >  line 348
> > <https://reviews.apache.org/r/28920/diff/13/?file=818526#file818526line348>
> >
> >     I would love to see another check for whether the scheduler is 
> > configured to allow docker mounts.  In fact, i'm not sure if 
> > `allow_docker_mounts` (or its replacement, based on another comment) should 
> > be plumbed anywhere else from here.
> >     
> >     Reasoning here is that only the cluster administrator should need to 
> > know what's on the slave command line.

I agree it should be checked here, and I'll add that in.  I think we need it in 
the task factory itself though as well so that in the case where the flag is 
added after job have been created, we don't keep creating jobs with mounts.  Do 
you have any thoughts on how to get the setting propegated into the 
ConfigurationManager?


> On Jan. 14, 2015, 1:03 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, line 125
> > <https://reviews.apache.org/r/28920/diff/13/?file=818523#file818523line125>
> >
> >     Should this instead be a list of allowed `ContainerType`s?  If we want 
> > this to support arbitrary container types, seems like a good litmus test 
> > for success is absense of the string 'docker' from code and comments.  Of 
> > course, there may be special cases that dictate otherwise.

This is for supporting mounting host paths into the docker containers.  I could 
rename this but I dont have a good idea of what the name would be.  
"allow_container_mounts" is confusing.  Maybe "allow_container_volumes"?


- Steve


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


On Jan. 13, 2015, 1:11 a.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 1:11 a.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 
>   config/legacy_untested_classes.txt 33c1d6eb4ea02e01b7002c2c2bae5a3858c8b0e5 
>   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
>   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
>   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
>   examples/vagrant/aurorabuild.sh 1e31f21998d02fd69ce0db88e6adb3d32cff67fd 
>   examples/vagrant/provision-dev-cluster.sh 
> 7af4b52a6876268a97630279221bb98d9b04efad 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 788ec254270bca074dae91829c7f4fccdc8f8bb0 
>   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/configuration/ConfigurationManager.java
>  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
>   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 
> f7d8977e42aa56188799400bf8e12a6886fb4a8f 
>   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/configuration/ConfigurationManagerTest.java
>  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
>   
> 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