> On Aug. 24, 2016, 1:21 p.m., Stephan Erb wrote: > > src/main/python/apache/thermos/core/process.py, line 437 > > <https://reviews.apache.org/r/51298/diff/2/?file=1482578#file1482578line437> > > > > 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 > > <https://reviews.apache.org/r/51298/diff/2/?file=1482578#file1482578line189> > > > > 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, > > Joshua Cohen > >
