> On Dec. 13, 2014, 1:59 a.m., Jay Buffington wrote:
> > I haven't had time to complete this review, but I wanted to give you what I 
> > have so far.  This is all fantastic and I really appreciate you doing this! 
> >  I'm excited to start using this implementation.
> > 
> > You should update configuration-reference.md and 
> > deploying-aurora-scheduler.md in the docs dir to explain these changes.  
> > You should state minimum mesos slave version as well explain minimum docker 
> > version required on the slaves.
> > 
> > You've made a bunch of changes to the executor and the runner to make them 
> > docker aware. I'd like those two components to not know about docker and I 
> > think we could really simplify this patch if we remove all that.  I think 
> > these changes are wholly unnecessary.  We should chat on IRC or Facetime 
> > about this.
> > 
> > I don't understand how users are managed in this world.  Docker images have 
> > their own /etc/passwd, so it seems to me that the unix user the mesos task 
> > runs as (same as aurora role) needs to exist inside that /etc/passwd.  If 
> > that's not the case I'm confused and want to understand what user (username 
> > and id) the task does run as inside and outside the container.
> > 
> > Allowing the aurora user to set volumes is a security nightmare.  before 
> > docker we're using unix permissions to control security.  We basically 
> > said: your aurora role is your unix user and that's all the permissions you 
> > get on a host.  The docker daemon runs as a priviledged user, so now you 
> > can tell docker to bind mount in files from all over the system that your 
> > unix user didn't previously have permissions to read.   So at a minimum, we 
> > should have a flag to disable the volumes feature with a big red warning 
> > flag in docs telling people what security issues they're signing up for 
> > whenever they enable it.

Oh, I forgot to mention the docs need to explain what requirements are on the 
docker image to run the docker executor in it.  Perhaps that's just python 2.7. 
 If that's the case, we should consider bind mounting (aka docker volumes) in 
(statically compiled?) py2.7 executable.


- Jay


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


On Dec. 11, 2014, 6:16 a.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2014, 6:16 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 script is in 
> examples/jobs/docker, as well as an example aurora file.
> 
> 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.
> 
> The thermos root path defaults to /var/run/thermos, however if a different 
> path is used, it must be passed to the scheduler via 
> `--thermos_observer_root=<some path>`
> 
> 
> Diffs
> -----
> 
>   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 5665c69cd7b49c3fd7345074c9f16a3b224496ab 
>   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
> 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
>   examples/jobs/docker/Dockerfile PRE-CREATION 
>   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
>   examples/jobs/docker/hello_docker.py PRE-CREATION 
>   examples/jobs/docker/thermos_executor.sh PRE-CREATION 
>   examples/vagrant/aurorabuild.sh 69983d0140b76c6869cd04e55d760f3e3a1e4262 
>   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/TaskScheduler.java 
> ead9d28100673440168a32d114ecaa15874978a6 
>   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/bin/thermos_runner.py 
> 647de2771f301b17de33d8b45198c211d2e84367 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/runner.py 
> 8aac6b50c66080abbb5308b367e9f74c487f42e3 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 5e54364a49a208bd5f19b9649633dc8feca591e9 
>   
> 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