Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-26 Thread Stephan Erb 

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


Ship it!




Ship It!

- Stephan Erb


On Aug. 26, 2016, 5:52 vorm., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 26, 2016, 5:52 vorm.)
> 
> 
> 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-26 Thread Aurora ReviewBot 

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



Master (50f47cc) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/tmpEZK_dq', 'binary', 
'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pants/setup/bootstrap-Linux-x86_64/1.1.0-rc7/bin/pants",
 line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  17 failed, 671 passed, 6 skipped, 1 warnings, 8 
error in 218.45 seconds 
 
FAILURE


06:36:39 04:23   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 26, 2016, 3:52 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 26, 2016, 3:52 a.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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 

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

(Updated Aug. 26, 2016, 3:52 a.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Latest round of review feedback.


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 (updated)
-

  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/core/runner.py 
dcfc190f7ed529e4f816e02b2d969077c60b008f 
  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 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 

Diff: https://reviews.apache.org/r/51298/diff/


Testing
---

./build-support/jenkins/build.sh
e2e tests


Thanks,

Joshua Cohen

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 


> On Aug. 25, 2016, 9:48 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, lines 165-168
> > 
> >
> > The `pop` is unnecessary. Luckily the Python interpreter is already 
> > protecting us from this mistake
> > 
> > ```
> > >>> def f(x, **kwargs):
> > ... print x
> > ...
> > >>> f(x=1, **{'x': 2})
> > Traceback (most recent call last):
> >   File "", line 1, in 
> > TypeError: f() got multiple values for keyword argument 'x'
> > ```

I'm not popping it for fear of it being set twice, I'm poppinig it because I 
want to be sure we pass `None` for the user value for this sandbox.


> On Aug. 25, 2016, 9:48 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, lines 253-257
> > 
> >
> > That `os.path.join(mesos_directory, container_path)` looks slightly 
> > strange. Either we have to stip the leading `/` of container path for this 
> > one as well, or the `join` is not necessary as (from the docs): 'If a 
> > component is an absolute path, all previous components are thrown away and 
> > joining continues from the absolute path component.'
> > 
> > Optional style nitpick: I feel the function could be easier to 
> > understand if we get rid of the list comprehension and move the computation 
> > of the two paths to the `for` loop below, so that declaration and usage are 
> > closer together.

So this was based on some confusion on my part. I was under the mistaken 
impression that Mesos would always mount volumes relative to `$MESOS_DIRECTORY` 
(thus why I was trying to join the paths). It was only working due to the fact 
that I was not stripping the leading slash from the container path, which as 
you point out causes us to throw away all previous path parts, meaning we were 
mounting that absolute path which is how Mesos *actually* mounts volumes into 
the container's mount namespace. Now that I've cleared that up everything works 
as expected without the os.path.join.

As for the rest, thanks for the nudge, that definitely was not the cleanest 
code.


> On Aug. 25, 2016, 9:48 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, lines 262-263
> > 
> >
> > If you feel fancy, you could shorten this via `lstrip`:
> > 
> > ```
> > >>> '/foo/bar'.lstrip('/')
> > 'foo/bar'
> > >>> 'foo/bar'.lstrip('/')
> > 'foo/bar'
> > ```

Ahh, nice, thanks!


> On Aug. 25, 2016, 9:48 p.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, lines 482-483
> > 
> >
> > We are now sourcing the thermos profile twice: Once within the 
> > container image and once on the outside. 
> > 
> > Probably does no harm.

I've cleaned this up so that we only source it once regardless.


- Joshua


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


On Aug. 25, 2016, 9:26 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 9:26 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Stephan Erb 


> On Aug. 25, 2016, 11:48 p.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, line 487
> > 
> >
> > In the taskfs_isolated case the `cwd` will contain a path within the 
> > container image. However here, we use the value while we are still in the 
> > host filesystem.  
> > 
> > Subprocess probably doesn't care if the `cwd` exists, but it feels 
> > weird.

This was posted on an old patch. All good now


- Stephan


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


On Aug. 25, 2016, 11:26 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 11:26 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Stephan Erb 

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



I really like the


src/main/python/apache/aurora/executor/common/sandbox.py (lines 159 - 162)


The `pop` is unnecessary. Luckily the Python interpreter is already 
protecting us from this mistake

```
>>> def f(x, **kwargs):
... print x
...
>>> f(x=1, **{'x': 2})
Traceback (most recent call last):
  File "", line 1, in 
TypeError: f() got multiple values for keyword argument 'x'
```



src/main/python/apache/aurora/executor/common/sandbox.py (lines 246 - 250)


That `os.path.join(mesos_directory, container_path)` looks slightly 
strange. Either we have to stip the leading `/` of container path for this one 
as well, or the `join` is not necessary as (from the docs): 'If a component is 
an absolute path, all previous components are thrown away and joining continues 
from the absolute path component.'

Optional style nitpick: I feel the function could be easier to understand 
if we get rid of the list comprehension and move the computation of the two 
paths to the `for` loop below, so that declaration and usage are closer 
together.



src/main/python/apache/aurora/executor/common/sandbox.py (lines 255 - 256)


If you feel fancy, you could shorten this via `lstrip`:

```
>>> '/foo/bar'.lstrip('/')
'foo/bar'
>>> 'foo/bar'.lstrip('/')
'foo/bar'
```



src/main/python/apache/thermos/core/process.py (lines 468 - 469)


We are now sourcing the thermos profile twice: Once within the container 
image and once on the outside. 

Probably does no harm.



src/main/python/apache/thermos/core/process.py (line 473)


In the taskfs_isolated case the `cwd` will contain a path within the 
container image. However here, we use the value while we are still in the host 
filesystem.  

Subprocess probably doesn't care if the `cwd` exists, but it feels weird.


- Stephan Erb


On Aug. 25, 2016, 11:26 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 11:26 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Aurora ReviewBot 

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



Master (50f47cc) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/tmpUYgdBQ', 'binary', 
'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pants/setup/bootstrap-Linux-x86_64/1.1.0-rc7/bin/pants",
 line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  16 failed, 672 passed, 6 skipped, 1 warnings, 8 
error in 168.86 seconds 
 
FAILURE


21:40:48 03:14   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 25, 2016, 9:26 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 9:26 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 

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

(Updated Aug. 25, 2016, 9:26 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Set cwd explicitly to the host sandbox for ths subprocess call.


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 (updated)
-

  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/core/runner.py 
dcfc190f7ed529e4f816e02b2d969077c60b008f 
  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 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 

Diff: https://reviews.apache.org/r/51298/diff/


Testing
---

./build-support/jenkins/build.sh
e2e tests


Thanks,

Joshua Cohen

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, line 437
> > 
> >
> > 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.
> 
> Joshua Cohen wrote:
> I'm not sure what you're getting at here? The `cwd` arg is passed on to 
> mesos-containerizer as the value for the `--working_directory` flag. That 
> value is used to `chdir` after the pivot_root/chroot logic is run. I've also 
> thoroughly verified that this directory is respected by the running task 
> (original by the `create_datafile` process in the e2e test, now replaced by 
> the `setup_env` process).
> 
> Stephan Erb wrote:
> Oh sorry, I highlighted the wrong line. I was referring to the cwd used 
> for the launching the subprocess.

Ahh! Gotcha! In practice it doesn't matter since the command executed is not 
concerned w/ cwd, but I'll update it to use the host sandbox directory just in 
case.


- Joshua


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


On Aug. 25, 2016, 7:28 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 7:28 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Stephan Erb 


> On Aug. 24, 2016, 1:21 p.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, line 437
> > 
> >
> > 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.
> 
> Joshua Cohen wrote:
> I'm not sure what you're getting at here? The `cwd` arg is passed on to 
> mesos-containerizer as the value for the `--working_directory` flag. That 
> value is used to `chdir` after the pivot_root/chroot logic is run. I've also 
> thoroughly verified that this directory is respected by the running task 
> (original by the `create_datafile` process in the e2e test, now replaced by 
> the `setup_env` process).

Oh sorry, I highlighted the wrong line. I was referring to the cwd used for the 
launching the subprocess.


> On Aug. 24, 2016, 1:21 p.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, lines 189-200
> > 
> >
> > 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?
> 
> Joshua Cohen wrote:
> It does not leak the containerizer launch call to the Thermos UI. The 
> observer takes a circuitous path, but essentially loads up the contents of 
> task.json from the sandbox which has the raw cmdline and not the value 
> generated here that's wrapped by `mesos-containerizer launch`.
> 
> As to your larger question, I'm not sure I follow your meaning. As part 
> of refactoring to properly support .thermos_profile, I moved the 
> mesos-specific logic from ProcessBase to Process, but that doesn't feel like 
> a major shift. Are you instead saying that everything Mesos-related should 
> live under apache.aurora rather apache.thermos? I can see the case for that, 
> but I feel like that split is artificial and just adds to complexity. We 
> don't have any intentions of supporting Thermos as a standalone process 
> manager and I'd rather not make this already complex change even more complex 
> by adding the need to plumb this command through to process from who knows 
> where ;).

With the new `wrapped_cmdline` you have achieved what I meant :)


- Stephan


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


On Aug. 25, 2016, 9:28 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 9:28 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Aurora ReviewBot 

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


Ship it!




Master (50f47cc) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 25, 2016, 7:28 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 7:28 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 
> 
> Diff: https://reviews.apache.org/r/51298/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> e2e tests
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 

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

(Updated Aug. 25, 2016, 7:28 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Fix broken test.


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 (updated)
-

  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/core/runner.py 
dcfc190f7ed529e4f816e02b2d969077c60b008f 
  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 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 

Diff: https://reviews.apache.org/r/51298/diff/


Testing
---

./build-support/jenkins/build.sh
e2e tests


Thanks,

Joshua Cohen

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Aurora ReviewBot 

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



Master (50f47cc) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/tmpZ0VUPU', 'binary', 
'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pants/setup/bootstrap-Linux-x86_64/1.1.0-rc7/bin/pants",
 line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  17 failed, 671 passed, 6 skipped, 1 warnings, 8 
error in 223.47 seconds 
 
FAILURE


19:24:08 04:37   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 25, 2016, 6:54 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 25, 2016, 6:54 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/core/runner.py 
> dcfc190f7ed529e4f816e02b2d969077c60b008f 
>   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

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, lines 
> > 117-124
> > 
> >
> > 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."

I did indeed consider this! However, in my testing the value of `MESOS_SANDBOX` 
matches the value of `MESOS_DIRECTORY` for our method of isolating the task's 
filesystem. I believe this is because the task is not actually specifying a 
`ContainerInfo`, but is instead mounting a `Volume` with an `Image` set.

I'll add a comment explaining as much.


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, line 257
> > 
> >
> > 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.

Done, also now raising an error in `__init__` if the value is `None`.


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, lines 266-270
> > 
> >
> > 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.

I *believe* that Mesos is already running the executor in its own mount 
namespace, so there's no chance of modifying the host's /etc/mtab.


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, lines 189-200
> > 
> >
> > 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?

It does not leak the containerizer launch call to the Thermos UI. The observer 
takes a circuitous path, but essentially loads up the contents of task.json 
from the sandbox which has the raw cmdline and not the value generated here 
that's wrapped by `mesos-containerizer launch`.

As to your larger question, I'm not sure I follow your meaning. As part of 
refactoring to properly support .thermos_profile, I moved the mesos-specific 
logic from ProcessBase to Process, but that doesn't feel like a major shift. 
Are you instead saying that everything Mesos-related should live under 
apache.aurora rather apache.thermos? I can see the case for that, but I feel 
like that split is artificial and just adds to complexity. We don't have any 
intentions of supporting Thermos as a standalone process manager and I'd rather 
not make this already complex change even more complex by adding the need to 
plumb this command through to process from who knows where ;).


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, line 420
> > 
> >
> > 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.

I fixed support for .thermos_profile and added coverage to the e2e tests to 
make sure it works.


> On Aug. 24, 2016, 11:21 a.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/process.py, line 437
> > 
> >
> > 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.

I'm not sure what you're getting at here? The `cwd` arg is passed on to 
mesos-containerizer as the value for the `--working_directory` flag. That value 
is used to `chdir` after the pivot_root/chroot logic is run. I've also 
thoroughly verified that this directory is respected by the running task 
(original by the

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 

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

(Updated Aug. 25, 2016, 6:54 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Minor cleanup missed in previous revision.


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 (updated)
-

  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/core/runner.py 
dcfc190f7ed529e4f816e02b2d969077c60b008f 
  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 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 

Diff: https://reviews.apache.org/r/51298/diff/


Testing
---

./build-support/jenkins/build.sh
e2e tests


Thanks,

Joshua Cohen

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-25 Thread Joshua Cohen 

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

(Updated Aug. 25, 2016, 6:51 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Review feedback:
- Fix support for .thermos_profile.
- Remove process_log_directory option in favor of cleaner separation of sandbox 
root/container sandbox root.


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 (updated)
-

  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/core/runner.py 
dcfc190f7ed529e4f816e02b2d969077c60b008f 
  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 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
0404d0e0a1a1054a5ed0c0b9f5f5be9fb3ecc113 

Diff: https://reviews.apache.org/r/51298/diff/


Testing
---

./build-support/jenkins/build.sh
e2e tests


Thanks,

Joshua Cohen

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-24 Thread Stephan Erb 

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




src/main/python/apache/aurora/executor/common/sandbox.py (line 36)


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)


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)


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)


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)


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)


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)


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)


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 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-23 Thread Aurora ReviewBot 

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


Ship it!




Master (c115ac6) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 23, 2016, 8: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, 8: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
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-23 Thread Joshua Cohen 


> On Aug. 22, 2016, 10:41 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, line 235
> > 
> >
> > ... then this if would not be needed.

One could potentially invoke the sandbox creation from elsewhere which could 
lead to the same case where we would fail to iterate. I figured it was better 
to make the handling of a `None` value explicit.


> On Aug. 22, 2016, 10:41 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, line 244
> > 
> >
> > When I remember your previous patch correctly, then the current working 
> > directory of our processes is set to the mesos_directory. This would imply 
> > that specifying an alternative sandbox mount point could break things.

Good catch. I cleaned up the cwd handling, which led to a bit of a rabbit hole 
of other fixes, but on the bright side, everything seems to work properly as 
far as bind mounting the sandbox directory into the taskfs.


- Joshua


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


On Aug. 22, 2016, 8:25 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 22, 2016, 8:25 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/test/python/apache/aurora/executor/common/test_sandbox.py 
> ce989b1ccda0f1bc7ba9e15dfe4be20116db3491 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 06601df3bc355a690ff1789b2e2e34484fadefe9 
>   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
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-22 Thread Stephan Erb 

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




src/main/python/apache/aurora/executor/aurora_executor.py (line 247)


If you return an empty list here...



src/main/python/apache/aurora/executor/common/sandbox.py (line 233)


... then this if would not be needed.



src/main/python/apache/aurora/executor/common/sandbox.py (line 242)


When I remember your previous patch correctly, then the current working 
directory of our processes is set to the mesos_directory. This would imply that 
specifying an alternative sandbox mount point could break things.


- Stephan Erb


On Aug. 22, 2016, 10:25 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 22, 2016, 10:25 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/test/python/apache/aurora/executor/common/test_sandbox.py 
> ce989b1ccda0f1bc7ba9e15dfe4be20116db3491 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 06601df3bc355a690ff1789b2e2e34484fadefe9 
>   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
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-22 Thread Maxim Khutornenko 

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


Ship it!




Ship It!

- Maxim Khutornenko


On Aug. 22, 2016, 8:25 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 22, 2016, 8:25 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/test/python/apache/aurora/executor/common/test_sandbox.py 
> ce989b1ccda0f1bc7ba9e15dfe4be20116db3491 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 06601df3bc355a690ff1789b2e2e34484fadefe9 
>   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
> 
>

Re: Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-22 Thread Aurora ReviewBot 

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


Ship it!




Master (9b34a40) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 22, 2016, 8:25 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51298/
> ---
> 
> (Updated Aug. 22, 2016, 8:25 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/test/python/apache/aurora/executor/common/test_sandbox.py 
> ce989b1ccda0f1bc7ba9e15dfe4be20116db3491 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 06601df3bc355a690ff1789b2e2e34484fadefe9 
>   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
> 
>

Review Request 51298: A few executor fixes for filesystem isolation:

2016-08-22 Thread Joshua Cohen 

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

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/test/python/apache/aurora/executor/common/test_sandbox.py 
ce989b1ccda0f1bc7ba9e15dfe4be20116db3491 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
06601df3bc355a690ff1789b2e2e34484fadefe9 
  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