Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen

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



@ReviewBot retry

- Joshua Cohen


On May 6, 2016, 3:42 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 3:42 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/features/containers.md 0d4791fb7b5ccae9c7b63341ab3dcff5bc62917c 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen

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



@ReviewBot retry

- Joshua Cohen


On May 6, 2016, 3:42 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 3:42 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/features/containers.md 0d4791fb7b5ccae9c7b63341ab3dcff5bc62917c 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Aurora ReviewBot

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



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

   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/user/10021/tmpBf8mXY', 
'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/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.80/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, 625 passed, 6 skipped, 1 warnings, 8 
error in 140.62 seconds 
 
FAILURE


   Waiting for background workers to finish.
16:06:25 03:14   [complete]
   FAILURE


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

- Aurora ReviewBot


On May 6, 2016, 3:42 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 3:42 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Stephan Erb

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


Ship it!




Ship It!

- Stephan Erb


On May 6, 2016, 5:42 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 5:42 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/features/containers.md 0d4791fb7b5ccae9c7b63341ab3dcff5bc62917c 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen

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

(Updated May 6, 2016, 3:42 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

checkstyle.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/features/containers.md 0d4791fb7b5ccae9c7b63341ab3dcff5bc62917c 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 
928ca9313b2c2062a322ba80b504a09c55e5377f 
  src/main/python/apache/aurora/executor/common/sandbox.py 
36f1eabedc3ae47b23d9ab2ac0ab7a576ea36fd7 
  

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Stephan Erb

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




docs/reference/configuration.md (line 606)


How about moving this section over here 
https://github.com/apache/aurora/blob/master/docs/features/containers.md? This 
should make it easier to discover for new users.



src/main/python/apache/aurora/config/thrift.py (line 154)


You missed one additional `unwrapped` reference here


- Stephan Erb


On May 6, 2016, 4:23 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 4:23 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Aurora ReviewBot

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



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

  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/BackupModule
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/StorageBackup$StorageBackupImpl
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/TemporaryStorage$TemporaryStorageFactory
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl$PendingRecovery
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$7
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$6
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$5
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$4
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$3
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$2
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$1
  Test coverage missing for org/apache/aurora/scheduler/HostOffer
  Test coverage missing for org/apache/aurora/scheduler/TaskVars
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerLifecycle$SchedulerCandidateImpl
  Test coverage missing for org/apache/aurora/scheduler/TierInfo
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerLifecycle$DefaultDelayedActions
  Test coverage missing for 
org/apache/aurora/scheduler/TierManager$TierManagerImpl$TierConfig
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$Counter
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$1
  Test coverage missing for 
org/apache/aurora/scheduler/TaskIdGenerator$TaskIdGeneratorImpl
  Test coverage missing for 
org/apache/aurora/scheduler/TierManager$TierManagerImpl
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl$1
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateStatusTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/MaintenanceModeTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/AbstractTEnumTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateActionTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/ResourceTypeHandler

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 10 mins 35.501 secs


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

- Aurora ReviewBot


On May 6, 2016, 2:23 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 2:23 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen

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



@ReviewBot retry

- Joshua Cohen


On May 6, 2016, 2:23 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 2:23 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
> ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
>   
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Aurora ReviewBot

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



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

  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$7
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$6
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$5
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$4
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$3
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$2
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$1
  Test coverage missing for org/apache/aurora/scheduler/HostOffer
  Test coverage missing for org/apache/aurora/scheduler/TaskVars
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerLifecycle$SchedulerCandidateImpl
  Test coverage missing for org/apache/aurora/scheduler/TierInfo
  Test coverage missing for 
org/apache/aurora/scheduler/SchedulerLifecycle$DefaultDelayedActions
  Test coverage missing for 
org/apache/aurora/scheduler/TierManager$TierManagerImpl$TierConfig
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$Counter
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$1
  Test coverage missing for org/apache/aurora/scheduler/SchedulerModule
  Test coverage missing for 
org/apache/aurora/scheduler/TaskIdGenerator$TaskIdGeneratorImpl
  Test coverage missing for 
org/apache/aurora/scheduler/TierManager$TierManagerImpl
  Test coverage missing for org/apache/aurora/scheduler/SchedulerModule$1
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl
  Test coverage missing for org/apache/aurora/scheduler/SchedulerServicesModule
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl$1
  Test coverage missing for org/apache/aurora/scheduler/TierModule
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateStatusTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/MaintenanceModeTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/AbstractTEnumTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateActionTypeHandler
  Test coverage missing for 
org/apache/aurora/scheduler/storage/db/typehandlers/ResourceTypeHandler

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 10 mins 5.964 secs


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

- Aurora ReviewBot


On May 6, 2016, 2:23 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 2:23 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen


> On May 6, 2016, 11:03 a.m., Stephan Erb wrote:
> > RELEASE-NOTES.md, line 32
> > 
> >
> > Isn't that a "removal" and should thus fit into the previous section? 
> > 
> > If you want to make it more prominent by using a separate section, we 
> > might consider switching to the format of http://keepachangelog.com: 
> > 
> > * `Added` for new features.
> > * `Changed` for changes in existing functionality.
> > * `Deprecated` for once-stable features removed in upcoming releases.
> > * `Removed` for deprecated features removed in this release.
> > * `Fixed` for any bug fixes.
> > * `Security` to invite users to upgrade in case of vulnerabilities.

I just moved it up to the removed section. I also added a note to the 
new/updated section about the change from `Container` to `Choice([Container, 
Docker, Mesos])`


> On May 6, 2016, 11:03 a.m., Stephan Erb wrote:
> > docs/reference/configuration.md, line 496
> > 
> >
> > That should probably be `image_id`.

Good catch, fixed.


> On May 6, 2016, 11:03 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 171
> > 
> >
> > Do I understand this correctly that `Container` is deprecated and we'd 
> > just want to specify `Docker` and `Mesos` here? 
> > 
> > If this analysis is correct, would you mind adding a comment and 
> > updating docs + changelog accordingly? Currently, the docs make it look 
> > like we would have to write `container=Container(mesos=Mesos(...))`.

Yep, I (hopefully) made this much more clear throughout.


> On May 6, 2016, 11:03 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/config/thrift.py, line 146
> > 
> >
> > That paramete `c` is never used. Looks like a bug to me. It doesn't 
> > crash because `unwrapped` is already defined in the scope of the 
> > surrounding function.

Fixed.


> On May 6, 2016, 11:03 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/config/thrift.py, line 150
> > 
> >
> > Extracting the body of this `if` into a separate function would make 
> > sure that the entire `create_container_config` function has a single level 
> > of abstraction. This should make it easier to understand.

Done.


- Joshua


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


On May 6, 2016, 2:23 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 6, 2016, 2:23 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Joshua Cohen

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

(Updated May 6, 2016, 2:23 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

- Make deprecation of specifying a `Container` object as the value of the 
container property on a job explicit in the docs and release notes.
- fix property name on AppcImage docs
- Refactor `create_container_config` slightly.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-06 Thread Stephan Erb

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



LGTM! A couple of minor things below.


RELEASE-NOTES.md (line 32)


Isn't that a "removal" and should thus fit into the previous section? 

If you want to make it more prominent by using a separate section, we might 
consider switching to the format of http://keepachangelog.com: 

* `Added` for new features.
* `Changed` for changes in existing functionality.
* `Deprecated` for once-stable features removed in upcoming releases.
* `Removed` for deprecated features removed in this release.
* `Fixed` for any bug fixes.
* `Security` to invite users to upgrade in case of vulnerabilities.



docs/reference/configuration.md (line 496)


That should probably be `image_id`.



src/main/python/apache/aurora/config/schema/base.py (line 171)


Do I understand this correctly that `Container` is deprecated and we'd just 
want to specify `Docker` and `Mesos` here? 

If this analysis is correct, would you mind adding a comment and updating 
docs + changelog accordingly? Currently, the docs make it look like we would 
have to write `container=Container(mesos=Mesos(...))`.



src/main/python/apache/aurora/config/thrift.py (line 146)


That paramete `c` is never used. Looks like a bug to me. It doesn't crash 
because `unwrapped` is already defined in the scope of the surrounding function.



src/main/python/apache/aurora/config/thrift.py (line 150)


Extracting the body of this `if` into a separate function would make sure 
that the entire `create_container_config` function has a single level of 
abstraction. This should make it easier to understand.


- Stephan Erb


On May 4, 2016, 8:41 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 8:41 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen

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

(Updated May 4, 2016, 6:41 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

Extracted golang version in build.sh


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 
928ca9313b2c2062a322ba80b504a09c55e5377f 
  src/main/python/apache/aurora/executor/common/sandbox.py 
36f1eabedc3ae47b23d9ab2ac0ab7a576ea36fd7 
  

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen


> On May 4, 2016, 4:47 p.m., Maxim Khutornenko wrote:
> > build-support/packer/build.sh, line 68
> > 
> >
> > Is it possible to carve out a version constant here?

Sure, will do.


- Joshua


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


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  lines 76-78
> > 
> >
> > Does this have to be bundled into this RB? We are still using this path 
> > in DEFAULT_CHECKPOINT_ROOT on the thermos side. Perhaps, have a more 
> > thorough cleanup in a separate patch?
> 
> Joshua Cohen wrote:
> I need to drop the mount for the e2e test to work. I can drop the mount 
> and leave the flag around unused for a later cleanup, but that will require 
> checkstyle suppressions. If you'd prefer I can send out a separate patch that 
> drops everything related to this arg and rebase this on top of it once it 
> lands.
> 
> Maxim Khutornenko wrote:
> Please, keep that cleanup on your radar.
> 
> Joshua Cohen wrote:
> I was about to file a ticket for this, but I'm not sure we *want* to 
> clean up this code from the Observer. In theory it's possible someone out 
> there has some long running tasks where they're running a more recent version 
> of the scheduler and the executor, but still have older executors running 
> that are writing to the global checkpoint root. In this case it makes sense 
> for the observer to continue performing lookups in both places.

I filed https://issues.apache.org/jira/browse/AURORA-1689 to track this one way 
or another (if we decide to maintain support we can just resolve as Won't Fix).


- Joshua


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


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  lines 76-78
> > 
> >
> > Does this have to be bundled into this RB? We are still using this path 
> > in DEFAULT_CHECKPOINT_ROOT on the thermos side. Perhaps, have a more 
> > thorough cleanup in a separate patch?
> 
> Joshua Cohen wrote:
> I need to drop the mount for the e2e test to work. I can drop the mount 
> and leave the flag around unused for a later cleanup, but that will require 
> checkstyle suppressions. If you'd prefer I can send out a separate patch that 
> drops everything related to this arg and rebase this on top of it once it 
> lands.
> 
> Maxim Khutornenko wrote:
> Please, keep that cleanup on your radar.

I was about to file a ticket for this, but I'm not sure we *want* to clean up 
this code from the Observer. In theory it's possible someone out there has some 
long running tasks where they're running a more recent version of the scheduler 
and the executor, but still have older executors running that are writing to 
the global checkpoint root. In this case it makes sense for the observer to 
continue performing lookups in both places.


- Joshua


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


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Aurora ReviewBot

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


Ship it!




Master (8a2fc4c) 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 May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Maxim Khutornenko

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



@ReviewBot retry

- Maxim Khutornenko


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
> ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
>   
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Maxim Khutornenko

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


Ship it!





build-support/packer/build.sh (line 68)


Is it possible to carve out a version constant here?


- Maxim Khutornenko


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Maxim Khutornenko


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  lines 76-78
> > 
> >
> > Does this have to be bundled into this RB? We are still using this path 
> > in DEFAULT_CHECKPOINT_ROOT on the thermos side. Perhaps, have a more 
> > thorough cleanup in a separate patch?
> 
> Joshua Cohen wrote:
> I need to drop the mount for the e2e test to work. I can drop the mount 
> and leave the flag around unused for a later cleanup, but that will require 
> checkstyle suppressions. If you'd prefer I can send out a separate patch that 
> drops everything related to this arg and rebase this on top of it once it 
> lands.

Please, keep that cleanup on your radar.


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java,
> >  line 84
> > 
> >
> > Shouldn't this and `DBImage` go into `DBContainer` now that 
> > `TaskConfig` does not have an `image` property?
> 
> Joshua Cohen wrote:
> I came up with something workable. Since there's no concept of a mesos 
> container in the database (and I don't think it makes sense to add a DB table 
> just so we can have a symmetrical mapping), I'm just mapping the image 
> directly to DbContainer. This is slightly different than the case for Docker, 
> where we map a DockerContainer. 
> 
> I'm not sure it's necessarily better though. Let me know what you think 
> ;).

Thanks, it's more logically fitting now.


- Maxim


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


On May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Aurora ReviewBot

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


Ship it!




Master (8a2fc4c) 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 May 4, 2016, 2:36 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:36 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> 5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen

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

(Updated May 4, 2016, 2:36 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

isort.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 
928ca9313b2c2062a322ba80b504a09c55e5377f 
  src/main/python/apache/aurora/executor/common/sandbox.py 
36f1eabedc3ae47b23d9ab2ac0ab7a576ea36fd7 
  

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Aurora ReviewBot

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



Master (8a2fc4c) is red with this patch.
  ./build-support/jenkins/build.sh

Collecting isort==4.0.0
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:315:
 SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name 
Indication) extension to TLS is not available on this platform. This may cause 
the server to present an incorrect TLS certificate, which can cause validation 
failures. For more information, see 
https://urllib3.readthedocs.org/en/latest/security.html#snimissingwarning.
  SNIMissingWarning
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:120:
 InsecurePlatformWarning: A true SSLContext object is not available. This 
prevents urllib3 from configuring SSL appropriately and may cause certain SSL 
connections to fail. For more information, see 
https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/thrift.py
 Imports are incorrectly sorted.
--- 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/thrift.py:before
   2016-05-04 14:21:12.312384
+++ 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/thrift.py:after
2016-05-04 14:29:13.507988
@@ -18,11 +18,11 @@
 from pystachio import Empty, Ref
 from twitter.common.lang import Compatibility
 
+from apache.aurora.config.schema.base import AppcImage as PystachioAppcImage
+from apache.aurora.config.schema.base import Container as PystachioContainer
+from apache.aurora.config.schema.base import DockerImage as 
PystachioDockerImage
 from apache.aurora.config.schema.base import (
-AppcImage as PystachioAppcImage,
-Container as PystachioContainer,
 Docker,
-DockerImage as PystachioDockerImage,
 HealthCheckConfig,
 Mesos,
 MesosContext,
ERROR: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/schema/base.py
 Imports are incorrectly sorted.
--- 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/schema/base.py:before
  2016-05-04 14:21:12.285384
+++ 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/config/schema/base.py:after
   2016-05-04 14:29:13.526268
@@ -15,8 +15,9 @@
 # Disable checkstyle for this entire file as it is a pystachio schema.
 # checkstyle: noqa
 
+from pystachio import Choice
+
 from apache.thermos.config.schema import *
-from pystachio import Choice
 
 
 # TODO(wickman) Bind {{mesos.instance}} to %shard_id%


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

- Aurora ReviewBot


On May 4, 2016, 2:13 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 2:13 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-04 Thread Joshua Cohen

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

(Updated May 4, 2016, 2:13 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

Rebase.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 8d5cbed2c627948c585241a8292a264e3d86120d 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
5f898a8d180cec6f3c02cb5b01673c56308ebd8a 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 e1ce6380e08178e0cd6b1f1651e49c7a4337fb94 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 
928ca9313b2c2062a322ba80b504a09c55e5377f 
  src/main/python/apache/aurora/executor/common/sandbox.py 
36f1eabedc3ae47b23d9ab2ac0ab7a576ea36fd7 
  

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Aurora ReviewBot

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



This patch does not apply cleanly against master (8a2fc4c), do you need to 
rebase?

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

- Aurora ReviewBot


On May 4, 2016, 1:45 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 1:45 a.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Joshua Cohen

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

(Updated May 4, 2016, 1:45 a.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

Review feedback and checkstyle issues.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
d2262f79edfde23eccd87bae7f1cf319b63b1103 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
ae97638fa544dd8f8afbaa19b1dd31f5a1dc43d8 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 
928ca9313b2c2062a322ba80b504a09c55e5377f 
  src/main/python/apache/aurora/executor/common/sandbox.py 
36f1eabedc3ae47b23d9ab2ac0ab7a576ea36fd7 
  

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Joshua Cohen


> On May 3, 2016, 11 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 146
> > 
> >
> > This can be removed, I guess.

Yep, missed that, thanks.


- Joshua


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


On May 3, 2016, 10:04 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 3, 2016, 10:04 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Joshua Cohen


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  lines 76-78
> > 
> >
> > Does this have to be bundled into this RB? We are still using this path 
> > in DEFAULT_CHECKPOINT_ROOT on the thermos side. Perhaps, have a more 
> > thorough cleanup in a separate patch?

I need to drop the mount for the e2e test to work. I can drop the mount and 
leave the flag around unused for a later cleanup, but that will require 
checkstyle suppressions. If you'd prefer I can send out a separate patch that 
drops everything related to this arg and rebase this on top of it once it lands.


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java,
> >  line 84
> > 
> >
> > Shouldn't this and `DBImage` go into `DBContainer` now that 
> > `TaskConfig` does not have an `image` property?

I came up with something workable. Since there's no concept of a mesos 
container in the database (and I don't think it makes sense to add a DB table 
just so we can have a symmetrical mapping), I'm just mapping the image directly 
to DbContainer. This is slightly different than the case for Docker, where we 
map a DockerContainer. 

I'm not sure it's necessarily better though. Let me know what you think ;).


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java,
> >  line 432
> > 
> >
> > Wouldn't a fake imageId suffice here?

Sure, will change.


> On May 4, 2016, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 201
> > 
> >
> > This name is no longer accurate given that we actually use 
> > ContainerInfo.Builder inside.

I just moved this code directly inside the calling method.


- Joshua


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


On May 3, 2016, 10:04 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 3, 2016, 10:04 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Maxim Khutornenko

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




src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 


Does this have to be bundled into this RB? We are still using this path in 
DEFAULT_CHECKPOINT_ROOT on the thermos side. Perhaps, have a more thorough 
cleanup in a separate patch?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 201)


This name is no longer accurate given that we actually use 
ContainerInfo.Builder inside.



src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
(line 83)


Shouldn't this and `DBImage` go into `DBContainer` now that `TaskConfig` 
does not have an `image` property?



src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
(line 422)


Wouldn't a fake imageId suffice here?


- Maxim Khutornenko


On May 3, 2016, 10:04 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 3, 2016, 10:04 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Stephan Erb

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




src/main/python/apache/aurora/config/schema/base.py (line 142)


This can be removed, I guess.


- Stephan Erb


On May 4, 2016, 12:04 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 4, 2016, 12:04 a.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 38470951e4482753fcada109ab12546a2fb146ce 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 9eadf70240738030593a819a4a248588c9d1c35b 
>   
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Aurora ReviewBot

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



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

:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java:54:8:
 error: Unused import - org.apache.aurora.gen.TaskConfig.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java:63:8:
 error: Unused import - 
org.apache.aurora.scheduler.storage.entities.ITaskConfig.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleTest'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/test.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 4 mins 6.199 secs


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

- Aurora ReviewBot


On May 3, 2016, 10:04 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated May 3, 2016, 10:04 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Joshua Cohen

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

(Updated May 3, 2016, 10:04 p.m.)


Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.


Changes
---

- Upgrade to pystachio 0.8.1 to pick up the new Choice type.
- Rework client configuration and thrift to make image a property of the Mesos 
container (made feasible by the pystachio Choice type).
- Remove the `thermos_run_as_job_role` arg.
- Remove the no longer necessary `thermos_observer_root` command line arg from 
the scheduler.


Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
https://issues.apache.org/jira/browse/AURORA-1636
https://issues.apache.org/jira/browse/AURORA-1637
https://issues.apache.org/jira/browse/AURORA-1638
https://issues.apache.org/jira/browse/AURORA-1639


Repository: aurora


Description
---

A few notes:

1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to a 
bug in parsing the docker_store_dir flag. Fixed here: 
https://reviews.apache.org/r/43451/ but has not been backported to Mesos 0.27. 
This means we can only launch tasks that use AppC images until we upgrade our 
Mesos dependency to 0.28.x. The good news is I've confirmed that launching 
tasks with Docker images *does* work by using Aurora linked against 0.27.x but 
running Mesos 0.28.x in Vagrant.
1. In order to work around the setuid issues (i.e. task is launched as root, 
but the executor cannot setuid because the role-user does not exist), I've 
mounted /etc/passwd and /etc/group into the container and added a new flag, 
`thermos_run_as_job_role`, to the scheduler. This flag is only used when 
launching a task with a filesystem image, and causes us to add 
`--execute-as-user ` to the thermos executor commandline.
1. The Mesos unified containerizer does not automatically create mount points 
in the filesystem from the image. It expects the full path to the mount to 
exist in the image. For /etc/passwd and /etc/groups this is not a problem, but 
for the announcer acls file it was. I ended up moving the announcer acl file 
into its own directory and mount that instead. In conjunction with this I also 
had to modify our http_example Dockerfile to explicitly create that mount 
point. A case could be made for sticking with the current path and just 
creating an empty file in the image, I felt that creating an empty directory 
was slightly less gross. This is tracked by 
https://issues.apache.org/jira/browse/MESOS-5229.
1. The AppC image for end to end tests is created by running 
[docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
image. The base box needed to be upgraded to add this utility. I haven't 
published the new base box yet even though I've updated the Vagrantfile to 
point to version 6. Once this review has been approved and I'm sure there's no 
further changes that need to be made I'll publish the base box before 
committing.


Diffs (updated)
-

  3rdparty/python/requirements.txt 666c4ae487332f01380cfce76f0d97e2c6049c8e 
  RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
  Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
38470951e4482753fcada109ab12546a2fb146ce 
  build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
  docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
  docs/reference/scheduler-configuration.md 
d2262f79edfde23eccd87bae7f1cf319b63b1103 
  examples/vagrant/announcer-auth.json  
  examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
PRE-CREATION 
  examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
084016abc169ed82b7ed00f5d14aea2e0ff38a49 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 32f2fa90b21189180e2bcd65a3cebf13f6551646 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
b325106c7f45b1ad1657221aaa39e3a428719ab0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
9aadcebf547bd1eb4b4e238507e27ae2b699f473 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
9eadf70240738030593a819a4a248588c9d1c35b 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
a7523c4b258030bcfb2e457b083242ffa865a98a 
  src/main/python/apache/aurora/config/schema/base.py 
00be8747d70dbf1cb370f09536588f8602d8fcce 
  src/main/python/apache/aurora/config/thrift.py 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Joshua Cohen


> On May 3, 2016, 8:39 p.m., Stephan Erb wrote:
> > RELEASE-NOTES.md, line 23
> > 
> >
> > How would I do the mounting? Via the global container mounts scheduler 
> > option?
> > 
> > So, once we have mounted the passwd and group file, is 
> > `thermos_run_as_job_role` still needed? Shouldn't that allow us to use the 
> > normal setuid of thermos?

> How would I do the mounting? Via the global container mounts scheduler option?

Yes (can see our aurora-scheduler.conf for the vagrant image for an example).

> So, once we have mounted the passwd and group file, is 
> thermos_run_as_job_role still needed? Shouldn't that allow us to use the 
> normal setuid of thermos?

You're right, it's actually not needed. The root cause of the problem that made 
me think this was required was that the `DockerDirectorySandbox` (now the 
`FileSystemImageSandbox`) did not take the user as a parameter, and instead 
always passed `None`. As part of adding support for `thermos_run_as_job_role` I 
added that arg to the constructor and didn't rethink the need for the scheduler 
arg entirely. Thanks for catching this!


- Joshua


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


On April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-03 Thread Stephan Erb

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




RELEASE-NOTES.md (line 23)


How would I do the mounting? Via the global container mounts scheduler 
option?

So, once we have mounted the passwd and group file, is 
`thermos_run_as_job_role` still needed? Shouldn't that allow us to use the 
normal setuid of thermos?


- Stephan Erb


On April 29, 2016, 6:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 6:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
>  501e6431f21822d9816952377546586da02ce42a 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/python/apache/aurora/config/schema/base.py 
> 00be8747d70dbf1cb370f09536588f8602d8fcce 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-02 Thread Joshua Cohen


> On May 2, 2016, 2:54 p.m., John Sirois wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 172
> > 
> >
> > The MesosJob is trending more and more towards a c-style union, 
> > different parts of the config are active depending on the setup and its not 
> > clear which except by reading docs carefully.  I put some effort towards 
> > supporting nicer pystachio evolution in an experiment documented in [this 
> > review feedback](https://reviews.apache.org/r/44745/) and there is a [new 
> > release](https://github.com/wickman/pystachio/releases) not used by Aurora 
> > yet that supports Choice fields.  It seems like work along these lines 
> > would be useful to stop the bleeding in general.  I'm not sure if that 
> > should of work should be coupled to this review though.
> 
> Joshua Cohen wrote:
> Oh, I didn't realize that `Choice` support landed in pystachio, that's 
> awesome! I think that making it explicit that you can specify either a 
> `Container` or an `Image` is, in general, a good thing. That said, thinking 
> it through further, the `Image` is not mutually exclusive with the container. 
> It's mutually exclusive with a docker container. What if instead of changing 
> job config such that you have to choose either a `Container` or an `Image`, I 
> instead just added a mesos property to the container struct and made `Image` 
> a property there? That more accurately reflects what's going on under the 
> covers (i.e. when specifying an `Image` on the job, what we're really doing 
> is using the MesosContainerizer and telling it to load that image).
> 
> John Sirois wrote:
> I think you mean this:
> ```thrift
> class AppcImage(Struct):
>   name = Required(String)
>   image_id = Required(String)
> 
> 
> class DockerImage(Struct):
>   name = Required(String)
>   tag = Required(String)
> 
> 
> class Mesos(Struct):
>   image = Image
> 
> 
> class Container(Struct):
>   docker = Docker
>   mesos = Mesos  
> ```
> 
> Which sounds ok.  I really don't like the `Container.(docker|mesos)` and 
> `Image.(appc|docker)` c-style union thing, but that could continue to be 
> deferred to a scoped upgrade to pystachio 0.8.1 + instroduce new Choice 
> fields deprecating the current situation.

Yes, that's what I'm suggesting. However, in retrospect, there's probably no 
reason *not* to pull in pystachio 0.8.1 to enable this to be a choice from the 
start. At least in that case the deprecation is only for Docker users, and 
people who adopt the unified containerizer won't have to deal with a 
deprecation down the line.

I can do the same on the thrift side as well, get rid of the TaskConfig.Image 
and add an optional Image to the existing MesosContainer.


- Joshua


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


On April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-02 Thread John Sirois


> On May 2, 2016, 8:54 a.m., John Sirois wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 172
> > 
> >
> > The MesosJob is trending more and more towards a c-style union, 
> > different parts of the config are active depending on the setup and its not 
> > clear which except by reading docs carefully.  I put some effort towards 
> > supporting nicer pystachio evolution in an experiment documented in [this 
> > review feedback](https://reviews.apache.org/r/44745/) and there is a [new 
> > release](https://github.com/wickman/pystachio/releases) not used by Aurora 
> > yet that supports Choice fields.  It seems like work along these lines 
> > would be useful to stop the bleeding in general.  I'm not sure if that 
> > should of work should be coupled to this review though.
> 
> Joshua Cohen wrote:
> Oh, I didn't realize that `Choice` support landed in pystachio, that's 
> awesome! I think that making it explicit that you can specify either a 
> `Container` or an `Image` is, in general, a good thing. That said, thinking 
> it through further, the `Image` is not mutually exclusive with the container. 
> It's mutually exclusive with a docker container. What if instead of changing 
> job config such that you have to choose either a `Container` or an `Image`, I 
> instead just added a mesos property to the container struct and made `Image` 
> a property there? That more accurately reflects what's going on under the 
> covers (i.e. when specifying an `Image` on the job, what we're really doing 
> is using the MesosContainerizer and telling it to load that image).

I think you mean this:
```thrift
class AppcImage(Struct):
  name = Required(String)
  image_id = Required(String)


class DockerImage(Struct):
  name = Required(String)
  tag = Required(String)


class Mesos(Struct):
  image = Image


class Container(Struct):
  docker = Docker
  mesos = Mesos  
```

Which sounds ok.  I really don't like the `Container.(docker|mesos)` and 
`Image.(appc|docker)` c-style union thing, but that could continue to be 
deferred to a scoped upgrade to pystachio 0.8.1 + instroduce new Choice fields 
deprecating the current situation.


- John


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


On April 29, 2016, 10:22 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 10:22 a.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-02 Thread Joshua Cohen


> On May 2, 2016, 2:54 p.m., John Sirois wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 172
> > 
> >
> > The MesosJob is trending more and more towards a c-style union, 
> > different parts of the config are active depending on the setup and its not 
> > clear which except by reading docs carefully.  I put some effort towards 
> > supporting nicer pystachio evolution in an experiment documented in [this 
> > review feedback](https://reviews.apache.org/r/44745/) and there is a [new 
> > release](https://github.com/wickman/pystachio/releases) not used by Aurora 
> > yet that supports Choice fields.  It seems like work along these lines 
> > would be useful to stop the bleeding in general.  I'm not sure if that 
> > should of work should be coupled to this review though.

Oh, I didn't realize that `Choice` support landed in pystachio, that's awesome! 
I think that making it explicit that you can specify either a `Container` or an 
`Image` is, in general, a good thing. That said, thinking it through further, 
the `Image` is not mutually exclusive with the container. It's mutually 
exclusive with a docker container. What if instead of changing job config such 
that you have to choose either a `Container` or an `Image`, I instead just 
added a mesos property to the container struct and made `Image` a property 
there? That more accurately reflects what's going on under the covers (i.e. 
when specifying an `Image` on the job, what we're really doing is using the 
MesosContainerizer and telling it to load that image).


- Joshua


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


On April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-02 Thread John Sirois

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



I've hung back from a detailed review base on Maxim's comment.  It may be that 
we need a bridge from old docker support to new unified container support that 
uses old(current)-style embedding of thermos in the image, but I'd like to hear 
that is definitely the case before repeating this tactic.


RELEASE-NOTES.md (line 18)


Can you just say that Aurora requires Mesos 0.28.0+ to use this feature?



src/main/python/apache/aurora/config/schema/base.py (line 172)


The MesosJob is trending more and more towards a c-style union, different 
parts of the config are active depending on the setup and its not clear which 
except by reading docs carefully.  I put some effort towards supporting nicer 
pystachio evolution in an experiment documented in [this review 
feedback](https://reviews.apache.org/r/44745/) and there is a [new 
release](https://github.com/wickman/pystachio/releases) not used by Aurora yet 
that supports Choice fields.  It seems like work along these lines would be 
useful to stop the bleeding in general.  I'm not sure if that should of work 
should be coupled to this review though.


- John Sirois


On April 29, 2016, 10:22 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 10:22 a.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-05-02 Thread Joshua Cohen


> On April 29, 2016, 11:39 p.m., Maxim Khutornenko wrote:
> > Following up on our offline conversation, it would be great to explore the 
> > feasibility of running executor outside of user image. This was one of the 
> > proposed [goals](http://markmail.org/message/g2xkh7nzzblokdgk) behind 
> > moving to unified containerizer and is likely [already 
> > possible](https://github.com/apache/mesos/blob/master/docs/container-image.md#executor-dependencies-in-a-container-image)
> >  with mesos. As it stands, there is not much that separates appc adoption 
> > from the existing docker implementation feature-wise. I am fine proceeding 
> > with this patch as an interim solution though as long as we identify the 
> > follow up work to explore the possibilities here.

Yes, I'll continue to investigate the feasibility of configuring the executor 
to be mounted via a Volume that specified an Image. That would allow us to 
decouple the executor's filesystem from the task's.

That said, I think that this patch paves the way for that work so I'd like it 
to move forward on its own if there are no objections.


- Joshua


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


On April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-04-29 Thread Maxim Khutornenko

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



Following up on our offline conversation, it would be great to explore the 
feasibility of running executor outside of user image. This was one of the 
proposed [goals](http://markmail.org/message/g2xkh7nzzblokdgk) behind moving to 
unified containerizer and is likely [already 
possible](https://github.com/apache/mesos/blob/master/docs/container-image.md#executor-dependencies-in-a-container-image)
 with mesos. As it stands, there is not much that separates appc adoption from 
the existing docker implementation feature-wise. I am fine proceeding with this 
patch as an interim solution though as long as we identify the follow up work 
to explore the possibilities here.

- Maxim Khutornenko


On April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
>  501e6431f21822d9816952377546586da02ce42a 
>   

Re: Review Request 46835: Add client and scheduler support for launching tasks using the Mesos unified containerizer

2016-04-29 Thread Aurora ReviewBot

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


Ship it!




Master (450d881) 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 April 29, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46835/
> ---
> 
> (Updated April 29, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora, John Sirois, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1636, AURORA-1637, AURORA-1638, and AURORA-1639
> https://issues.apache.org/jira/browse/AURORA-1636
> https://issues.apache.org/jira/browse/AURORA-1637
> https://issues.apache.org/jira/browse/AURORA-1638
> https://issues.apache.org/jira/browse/AURORA-1639
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few notes:
> 
> 1. It's not possible to configure Mesos 0.27.x to launch docker tasks due to 
> a bug in parsing the docker_store_dir flag. Fixed here: 
> https://reviews.apache.org/r/43451/ but has not been backported to Mesos 
> 0.27. This means we can only launch tasks that use AppC images until we 
> upgrade our Mesos dependency to 0.28.x. The good news is I've confirmed that 
> launching tasks with Docker images *does* work by using Aurora linked against 
> 0.27.x but running Mesos 0.28.x in Vagrant.
> 1. In order to work around the setuid issues (i.e. task is launched as root, 
> but the executor cannot setuid because the role-user does not exist), I've 
> mounted /etc/passwd and /etc/group into the container and added a new flag, 
> `thermos_run_as_job_role`, to the scheduler. This flag is only used when 
> launching a task with a filesystem image, and causes us to add 
> `--execute-as-user ` to the thermos executor commandline.
> 1. The Mesos unified containerizer does not automatically create mount points 
> in the filesystem from the image. It expects the full path to the mount to 
> exist in the image. For /etc/passwd and /etc/groups this is not a problem, 
> but for the announcer acls file it was. I ended up moving the announcer acl 
> file into its own directory and mount that instead. In conjunction with this 
> I also had to modify our http_example Dockerfile to explicitly create that 
> mount point. A case could be made for sticking with the current path and just 
> creating an empty file in the image, I felt that creating an empty directory 
> was slightly less gross. This is tracked by 
> https://issues.apache.org/jira/browse/MESOS-5229.
> 1. The AppC image for end to end tests is created by running 
> [docker2aci](https://github.com/appc/docker2aci) on our http_example docker 
> image. The base box needed to be upgraded to add this utility. I haven't 
> published the new base box yet even though I've updated the Vagrantfile to 
> point to version 6. Once this review has been approved and I'm sure there's 
> no further changes that need to be made I'll publish the base box before 
> committing.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 7a37d0d69f688bece624628fe5b98efc85d506a2 
>   Vagrantfile 3f126ee348d0f95d6f159b62280de79f41e87e2e 
>   build-support/packer/build.sh 76197c31c365aa3d8a67049da40b2976c1e25d22 
>   docs/reference/configuration.md 9fcfdfcd9ab793e888ca2bba2035d5122142a5ab 
>   docs/reference/scheduler-configuration.md 
> d2262f79edfde23eccd87bae7f1cf319b63b1103 
>   examples/vagrant/announcer-auth.json  
>   examples/vagrant/mesos_config/etc_mesos-slave/appc_store_dir PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_providers PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/image_provisioner_backend 
> PRE-CREATION 
>   examples/vagrant/mesos_config/etc_mesos-slave/isolation PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 084016abc169ed82b7ed00f5d14aea2e0ff38a49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  32f2fa90b21189180e2bcd65a3cebf13f6551646 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
>  501e6431f21822d9816952377546586da02ce42a 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> b325106c7f45b1ad1657221aaa39e3a428719ab0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 9aadcebf547bd1eb4b4e238507e27ae2b699f473 
>   src/main/python/apache/aurora/config/schema/base.py 
> 00be8747d70dbf1cb370f09536588f8602d8fcce 
>   src/main/python/apache/aurora/config/thrift.py 
> 928ca9313b2c2062a322ba80b504a09c55e5377f 
>   src/main/python/apache/aurora/executor/common/sandbox.py 
>