Advertising

src/main/python/apache/aurora/executor/common/sandbox.py (line 36)
<https://reviews.apache.org/r/51298/#comment213183>
Please extend this doc string slightly to mention that this path is within
the host filesystem.
src/main/python/apache/aurora/executor/bin/thermos_executor_main.py (lines 117
- 124)
<https://reviews.apache.org/r/51298/#comment213205>
Have you considered reading that value from the `MESOS_SANDBOX` environment
variable?
From the docs: "MESOS_SANDBOX: Path to the mapped sandbox inside of the
container (determined by the agent flag sandbox_directory) for either mesos
container with image or docker container. For the case of command task without
image specified, it is the path to the sandbox on the host filesystem, which is
identical to MESOS_DIRECTORY. MESOS_DIRECTORY is always the sandbox on the host
filesystem."
src/main/python/apache/aurora/executor/common/sandbox.py (line 250)
<https://reviews.apache.org/r/51298/#comment213204>
I believe we don't need that `or mesos_directory` here. It should never
happen that sanbox_mount_point is None. And if it happens, it is better to
crash early rather then running into undefined behaviour later on.
src/main/python/apache/aurora/executor/common/sandbox.py (lines 259 - 263)
<https://reviews.apache.org/r/51298/#comment213203>
We are executing this as root on the host filesystem. It might therefore
make sense to pass `-n` here to prevent that the `/etc/mtab` is modified.
src/main/python/apache/thermos/core/process.py (lines 189 - 200)
<https://reviews.apache.org/r/51298/#comment213208>
The `ProcessBase` class is normaly oblivious to the existance of Mesos. Do
you feel it would make sense to inline this special case at the call-side?
I haven't checked that, but I suppose in its current place we would leak
the containerizer launch call in the Thermos UI?
src/main/python/apache/thermos/core/process.py (line 420)
<https://reviews.apache.org/r/51298/#comment213181>
This will point to a wrong path in the `taskfs_isolated` case as the
profile will be sourced by bash before calling the containerize launch command.
At that point in time we are still in the host filesystem.
Might be worthwhile to add a test to check that it is working correctly.
src/main/python/apache/thermos/core/process.py (line 437)
<https://reviews.apache.org/r/51298/#comment213175>
In the `taskfs_isolated` case, `cwd` will be set to a path within the
container, but we use it before the pivot root is completed. This is a problem
similar to the thermos_profile issue mentioned above.
src/main/python/apache/thermos/runner/thermos_runner.py (lines 139 - 144)
<https://reviews.apache.org/r/51298/#comment213170>
Can this be changed in a meaningful way? I have always assumed that the
observer UI has the hardcoded assumption that logs can be found within
`sandbox/.logs`.
- Stephan Erb
On Aug. 23, 2016, 10:35 p.m., Joshua Cohen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2016, 10:35 p.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> - Add an option to skip the groupadd/useradd calls into the task's filesystem.
> - Mount any configured volumes into the task's filesystem.
> - Clean up http server script used by appc e2e tests.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/executor/aurora_executor.py
> dde19a6914c7c7b2178220707f242f61f11f38bd
> src/main/python/apache/aurora/executor/bin/thermos_executor_main.py
> 65a495d5c50d91b38c4328bab3bfec667f6a7ba9
> src/main/python/apache/aurora/executor/common/sandbox.py
> 5f091af7636bd94f028f15d63437e305b02f741c
> src/main/python/apache/aurora/executor/thermos_task_runner.py
> 1d713ca3d81a2e7be88b787dfcab328d887be24c
> src/main/python/apache/thermos/core/process.py
> a296fa715ef6fbb8d5feae356914334437f353f1
> src/main/python/apache/thermos/runner/thermos_runner.py
> 441bacdfe93b1805a03a1216762c74db810a9540
> src/test/python/apache/aurora/executor/common/test_sandbox.py
> ce989b1ccda0f1bc7ba9e15dfe4be20116db3491
> src/test/python/apache/aurora/executor/test_thermos_executor.py
> 06601df3bc355a690ff1789b2e2e34484fadefe9
> src/test/python/apache/thermos/core/test_process.py
> 759f783202803c296ce19bb64c59cbe896d40a43
> src/test/sh/org/apache/aurora/e2e/http/http_example.aurora
> b69ddf129c0015a878b089d85d731bc0c26fd55c
> src/test/sh/org/apache/aurora/e2e/run-server.sh
> 76939888bed2e8138671d97f7bc56fd5641008e4
>
> Diff: https://reviews.apache.org/r/51298/diff/
>
>
> Testing
> -------
>
> ./build-support/jenkins/build.sh
> e2e tests
>
>
> Thanks,
>
> Joshua Cohen
>
>