> 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.
> 
> Steve Niemitz wrote:
>     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?

> in the case where the flag is added after job have been created, we don't 
> keep creating jobs with mounts

Hmm, that's a tough one.  I wonder if we should leave it up to the 
administrator to clean up when they make flag changes with implications like 
that.  Open to ideas, but this seems difficult to chase after in general.  We 
also currently lack a means to communicate back to the user that a job is 
incompatible with the scheduler configuration.

> Do you have any thoughts on how to get the setting propegated into the 
> ConfigurationManager?

We have some precedent for defining them as static fields in classes like this, 
but have started to backpedal on that as it is hostile to unit testing.  
Perhaps you can create a `ConfigurationSettings` class, to be injected into 
`SchedulerThriftInterface` and passed to `ConfigurationManager`?  One thing to 
keep in mind - there are two primary entrypoints to `ConfigurationManager` - 
one for thrift, and the other for storage recovery.  The thrift entrypoint 
tends to do validation and storage recovery mostly does backfilling of fields, 
to make sure that we continue to run jobs we have previously accepted.


- Bill


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


On Jan. 14, 2015, 7:14 p.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 7:14 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 
>   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