> On May 26, 2016, 12:17 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, lines 208-230
> > <https://reviews.apache.org/r/47853/diff/1/?file=1393961#file1393961line208>
> >
> >     Seeing this spelled out, I feel like we should not have such code in 
> > Thermos. It is rather low-level, hard to get compeltely right, and can 
> > impose security risks if done wrong.
> >     
> >     Mesos already has to maintain a version of the `chroot`/`pivot_root` 
> > mechanism. We should try to leverage it. Otherwise, this will be an up-hill 
> > battle that we cannot win in the long run, especially given the limited 
> > number of Aurora committers
> >     
> >     I see a couple options:
> >     
> >     a) We get the Mesos guys to export their `pivot_root` code so that all 
> > we have to do is call a single Mesos command, just like it could currently 
> > be done for the `mesos-fetcher` or the entire `mesos-containerizer`:
> >     
> >     ```
> >     vagrant@aurora:~$ /usr/libexec/mesos/mesos-containerizer 
> >     Usage: /usr/libexec/mesos/mesos-containerizer <subcommand> [OPTIONS]
> >     
> >     Available subcommands:
> >         help
> >         launch
> >         mount
> >     ```
> >     
> >     b) We live with the fact that we'll always run Thermos within the 
> > container. This will require Python, but we could drop the dependency on 
> > other Mesos dependencies by migrating to the new executor HTTP API.
> >     
> >     
> >     c) We stick to the `MesosContainer.image` for the user container, but 
> > use a `Volume` `image` in order to mount a minimal container that is just 
> > packaging Thermos and its dependencies. We could then launch the executor 
> > from the mounted volume such as `/thermos/bin/thermox_executor.sh` (with 
> > appropriate P`YTHON_PATH` and `LD_LIBRARY_PATH`). The user could would 
> > still be running using the user-image.
> >     
> >     
> >     Approaches b) and c) keep the door open for us to run the executor as 
> > an un-priviledged user. This has security advantages and would als make it 
> > easier for users to leverage security features such as the new Mesos Linux 
> > Capabilities 
> > https://docs.google.com/document/d/1YiTift8TQla2vq3upQr7K-riQ_pQ-FKOCOsysQJROGc/edit?pref=2&pli=1

Just to capture offline discussion here: I've reached out to mesos-dev list and 
they're open to the idea of adding a subcommand to mesos-containerizer that 
would be responsible for isolating a task's filesystem.

We hope to work on and contribute this patch next week during Mesoscon.

Given that, I'm going to hold off on this review for the time being.


- Joshua


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


On May 25, 2016, 9:18 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47853/
> -----------------------------------------------------------
> 
> (Updated May 25, 2016, 9:18 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This changes the approach to launching tasks with filesystem images in the 
> unified containerizer. Instead of adding an `Image` to the `MesosContainer`, 
> we instead add the task filesystem as a `Volume` with an associated image. 
> This image is mounted in the mesos directory under the `taskfs` path. The 
> executor, on start up does the following:
> 
> 1. Creates user/group under the taskfs root.
> 2. `pivot_root`s into the taskfs, while bind mounting the sandbox under that 
> root as well as mounting procfs.
> 3. From there, task execution is essentially unchanged minus some slight 
> changes to the environment depending on whether we're running in a pivoted 
> root.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> a99889c1f2d9e10825f87ea669532ad78641880f 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 3d9e706de564df5e24cb34265bebc0db1cad11a0 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/python/apache/aurora/executor/common/sandbox.py 
> be1deba6219462c9fdaaf07a583851b85fe974bf 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 3896e3841562600379705dbf78a6f62728246348 
>   src/main/python/apache/thermos/core/BUILD 
> 1094664e112cc71af37835f32037e9eb6d047202 
>   src/main/python/apache/thermos/core/process.py 
> 1791b5ff9a36eef7470bef9a6ebbafaf0ab05ca3 
>   src/main/python/apache/thermos/core/runner.py 
> 3ebf86ebd12ed3b68f543d4b9a45615e4681ba7f 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> 0d06e8e2ac78d26ba8f63744853eb5ce3f6aced6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/python/apache/aurora/executor/common/test_sandbox.py 
> e47d9b8822deb36cb9cfa0554ef89d6cda80f3e9 
>   src/test/python/apache/thermos/core/test_process.py 
> 77f644c09116266ce02479b9a80403aa68767bd6 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> 6fdea3d28760f59235c51c5b6913d2ee0172ef1a 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile.netcat PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> 219c40fb94561f0a390cac16e643bf4332c51aad 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 08553e4f48f137e0455ad07287086311171c06bd 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> 8b3a50ba6de992560593987f3db254baa4d29a41 
>   src/test/sh/org/apache/aurora/e2e/run-server.sh PRE-CREATION 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> abe0ca75c6a2c0ace15fce68ad0e5c9aa98193a4 
> 
> Diff: https://reviews.apache.org/r/47853/diff/
> 
> 
> Testing
> -------
> 
> Lots of manual testing, e2e tests, etc.
> 
> I didn't add much coverage on the thermos side of things because it seemed 
> like this was better served by the e2e tests than by doing a bunch of 
> subprocess.check_call mocking. On the e2e front I created a new Dockerfile 
> that sets up a much slimmer filesystem image that explicitly does not include 
> python to ensure that the executor's filesystem is truly isolated.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Reply via email to