Re: Review Request 28920: Add support for docker containers to aurora

2015-01-22 Thread Jay Buffington

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



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment114008

Can you set force_pull_image to true here.  I can't imagine why you would 
ever want that to be false. 
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1028


- Jay Buffington


On Jan. 16, 2015, 12:08 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 16, 2015, 12:08 a.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample aurora file is in 
 examples/jobs/docker.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 More information is in subsequent review request comments.
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 08ba1cdf88b712de22c26c04443079282db59ef9 
   config/legacy_untested_classes.txt 33c1d6eb4ea02e01b7002c2c2bae5a3858c8b0e5 
   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/vagrant/aurorabuild.sh 1e31f21998d02fd69ce0db88e6adb3d32cff67fd 
   examples/vagrant/provision-dev-cluster.sh 
 7af4b52a6876268a97630279221bb98d9b04efad 
   examples/vagrant/upstart/aurora-scheduler.conf 
 788ec254270bca074dae91829c7f4fccdc8f8bb0 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 f7d8977e42aa56188799400bf8e12a6886fb4a8f 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/config/schema_base.py 
 f9143cc1b83143d6147f59d90c79435d055d0518 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   src/main/resources/scheduler/assets/configSummary.html 
 28878908b0c9381e366b71a3135dfc28c542a890 
   src/main/resources/scheduler/assets/js/services.js 
 b744f375411e09b7f776e4a05ee5961227143439 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
 876e173ccbac04e4a06a245648c7c6af15eaaa92 
   
 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 7eafe074b686d55ad96018006cf4acfa823513c3 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  ad9126c32893080e128d086ea3bfd7ad23d27b89 
   src/test/python/apache/aurora/client/cli/test_status.py 
 e531fa06e508d9792af51c62e67120c21baa7a81 
   src/test/python/apache/aurora/executor/test_thermos_executor.py 
 503e62f4cac872b14f6985b5bccc3e4dfcf81789 
 
 Diff: https://reviews.apache.org/r/28920/diff

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-05 Thread Jay Buffington


 On Dec. 13, 2014, 1:59 a.m., Jay Buffington wrote:
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
  279
  https://reviews.apache.org/r/28920/diff/2/?file=789365#file789365line279
 
  I would do away with this TaskConfig hasProcesses field.  You should 
  just use config.isSetExecutorConfig() (and of course not set executor 
  config in the python client when there are no processes.)
 
 Steve Niemitz wrote:
 I'm going to remove support for running the container directly w/o the 
 executor for now.  Without the job registering with the observer, aurora 
 thinks the job is lost and will GC it or abandon it.  We can add this as a 
 to-do in the future.

I'm +1 on this.  I think running without the executor is of dubious value 
anyway.  I hadn't considered the issue with the GC.  Thanks for pointing this 
out!


- Jay


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


On Dec. 26, 2014, 9:05 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Dec. 26, 2014, 9:05 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample script is in 
 examples/jobs/docker, as well as an example aurora file.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 The thermos root path defaults to /var/run/thermos, however if a different 
 path is used, it must be passed to the scheduler via 
 `--thermos_observer_root=some path`
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 5665c69cd7b49c3fd7345074c9f16a3b224496ab 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/vagrant/aurorabuild.sh 69983d0140b76c6869cd04e55d760f3e3a1e4262 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 ead9d28100673440168a32d114ecaa15874978a6 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/config/schema_base.py 
 f9143cc1b83143d6147f59d90c79435d055d0518 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
 876e173ccbac04e4a06a245648c7c6af15eaaa92 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
   src/test/python/apache/aurora/executor/test_thermos_executor.py 
 503e62f4cac872b14f6985b5bccc3e4dfcf81789 
 
 Diff: https://reviews.apache.org/r/28920/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Steve Niemitz
 




Re: Review Request 28920: Add support for docker containers to aurora

2014-12-16 Thread Jay Buffington

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


There some issues here with usability that maybe this patch doesn't need to 
address, but should be tracked in a jira if it isn't.

The main usability issues are:

* The user should be able to go to the web ui and click show config to 
show the docker image the user specified.
* When the docker pull fails (user specified bogus image) the user sees 
Abnormal executor termination
* when docker run fails (say, dockerd isn't running) the user sees 
Unregistered executor
* the instance state in the webui is ASSIGNED (or is it ASSINGING?) when 
the docker pull is happening, which could take a while

For the last three of these I believe mesos needs to be modified to send the 
scheduler more state.

Related to this is that when the docker pull takes longer than 
transient_state_task_timeout aurora will give up and fail the task.

- Jay Buffington


On Dec. 16, 2014, 9:19 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Dec. 16, 2014, 9:19 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample script is in 
 examples/jobs/docker, as well as an example aurora file.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 The thermos root path defaults to /var/run/thermos, however if a different 
 path is used, it must be passed to the scheduler via 
 `--thermos_observer_root=some path`
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 5665c69cd7b49c3fd7345074c9f16a3b224496ab 
   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
   examples/jobs/docker/Dockerfile PRE-CREATION 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/jobs/docker/hello_docker.py PRE-CREATION 
   examples/jobs/docker/thermos_executor.sh PRE-CREATION 
   examples/vagrant/aurorabuild.sh 69983d0140b76c6869cd04e55d760f3e3a1e4262 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 ead9d28100673440168a32d114ecaa15874978a6 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/bin/thermos_runner.py 
 647de2771f301b17de33d8b45198c211d2e84367 
   src/main/python/apache/thermos/config/schema_base.py 
 f9143cc1b83143d6147f59d90c79435d055d0518 
   src/main/python/apache/thermos/core/runner.py 
 8aac6b50c66080abbb5308b367e9f74c487f42e3 
   src/main/python/apache/thermos/observer/task_observer.py 
 cd528dcca3f5a330359cf38005f3a1a0329a4886 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 5e54364a49a208bd5f19b9649633dc8feca591e9 
   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
 876e173ccbac04e4a06a245648c7c6af15eaaa92 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 ddcb511d108220ab5e4efcf3496458f7ab4a20c2 
   src/test/python/apache/aurora/executor/test_thermos_executor.py 
 503e62f4cac872b14f6985b5bccc3e4dfcf81789 
 
 Diff: https

Re: Review Request 28920: Add support for docker containers to aurora

2014-12-16 Thread Jay Buffington

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


Sorry for the multiple reviews.  There is a lot here.  Maybe we should be 
having these architecture discussions in Jira?

I propose you remove the wrapper script all together.  To do that we need 
alternative ways to implement the features enabled by --execute-as-container 
and --dockerize flags.

My read of --dockerize is that it was introduced as a solution to the problem I 
described in 
https://issues.apache.org/jira/browse/AURORA-633?focusedCommentId=14134299page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14134299.
  I believe the long term fix to this problem is to run an observer per 
executor.  See AURORA-708: allow thermos observer to be launched from within 
aurora executor

Until then, I propose you replace all that dockerize and host_sandbox and 
host_log_dir stuff with a change to the  `_initialize_ckpt_header` function of  
`/thermos/core/runner.py` to set RunnerHeader's sandbox value to 
`os.environ.get('MESOS_DIRECTORY') || self._sandbox`

The --execute-as-container flag I *think* is used to tell the runner not to do 
a setuid.  How is that different than starting the runner with --setuid=root 
(which already exists)?   Also, I say we always run the task as nobody inside 
the container.  The executor can check if we're inside a docker container (test 
for the existance of /.dockerinit file) and call runner with --setuid=nobody.

An alternative to having a wrapper script is to allow the administrator to 
start the scheduler with a -docker_executor_launch_command flag where they 
inline a bash wrapper script.  This is a little gnarly to manage because you 
end up with shell quoting frustrations.


src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
https://reviews.apache.org/r/28920/#comment108355

This is an optional arg with no default, and you throw an NPE in 
ExecutorSettings if it isn't set.


- Jay Buffington


On Dec. 16, 2014, 9:19 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Dec. 16, 2014, 9:19 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample script is in 
 examples/jobs/docker, as well as an example aurora file.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 The thermos root path defaults to /var/run/thermos, however if a different 
 path is used, it must be passed to the scheduler via 
 `--thermos_observer_root=some path`
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 5665c69cd7b49c3fd7345074c9f16a3b224496ab 
   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
   examples/jobs/docker/Dockerfile PRE-CREATION 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/jobs/docker/hello_docker.py PRE-CREATION 
   examples/jobs/docker/thermos_executor.sh PRE-CREATION 
   examples/vagrant/aurorabuild.sh 69983d0140b76c6869cd04e55d760f3e3a1e4262 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 ead9d28100673440168a32d114ecaa15874978a6 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py

Re: Review Request 28198: Add an example on how to build with Docker.

2014-12-12 Thread Jay Buffington

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


This is *awesome*.  Using docker to do aurora builds in and to ship an aurora 
runtime is fantastic.

My major issue with this commit is that you're unnecessarily putting build 
dependencies (aurora source, python-dev, git, etc) into a runtime container.

I suggest a slightly different approach than you've taken here: there should be 
two Dockerfiles: one for build and one for runtime.  Also, I don't think they 
should be examples, let's hook docker up to the CI process and ship a runnable 
docker image for every commit of aurora!

The build Dockerfile should produce a docker image which we can run the build 
in.  It will contain all build time dependencies (gradle, python-dev, etc).  
When a developer wants to do the build they will simply run docker run 
aurora/build This will use volume mounts to import the code into the container 
and will write out a number of jar and pex files back to the developer's 
workspace.  This is really useful because now we have a programatic way (in the 
Dockerfile) to express *all* build time dependencies.

To build a runtime container, we ship another Dockerfile which installs all the 
runtime deps (java 1.7, etc) and uses Dockerfile's ADD to copy the jar and pex 
files from the developer's workspace into the container.  We run build this 
docker container on every commit through a CI process and ship it to a docker 
registry.  Then to run, users just have to do docker run aurora/aurora

- Jay Buffington


On Dec. 11, 2014, 6:32 p.m., Tony Dong wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28198/
 ---
 
 (Updated Dec. 11, 2014, 6:32 p.m.)
 
 
 Review request for Aurora, Benjamin Staffin, Kevin Sweeney, Bill Farner, and 
 Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add an example on how to build with Docker.
 
 
 Diffs
 -
 
   README.md fe46b4f071e1cc8923ac52ea461b66456709eb5d 
   examples/docker/Dockerfile PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/28198/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Tony Dong
 




Re: Review Request 28920: Add support for docker containers to aurora

2014-12-12 Thread Jay Buffington
/#comment107925

info is too verbose here.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment107903

This is confusing to the user and I think unnecessary.  It seems like the 
user should be able to use whatever wrapper script they want (why does it have 
to end in .sh?)  

The requirements of what the wrapper script must do should be explained in 
the deploying-aurora doc



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment107905

To make this log message useful to an operations person reading it, it 
should explain which job this message pertains to.



src/main/python/apache/aurora/config/thrift.py
https://reviews.apache.org/r/28920/#comment107927

s/Task/Job/

task is a mesos construct, job is an aurora construct.



src/main/python/apache/aurora/executor/aurora_executor.py
https://reviews.apache.org/r/28920/#comment107928

I think what you're trying to accomplish here with all these changes to the 
executor can be done with a much more trival and non-docker specific change: 
just make the DefaultSandbox directory configurable via an env var.

Just change SANDBOX_NAME = 'sandbox' to something like this:

SANDBOX_NAME = os.getenv('AURORA_SANDBOX') or 'sandbox'

 Then, of course, you'll have to set that in the protobuf message you send 
to mesos in executorinfo.commandinfo.value.



src/main/python/apache/aurora/executor/thermos_task_runner.py
https://reviews.apache.org/r/28920/#comment107931

I'm lost with these role changes to the thermos_runner.  Could you give me 
an overview of what this does and why it is necessary?



src/main/python/apache/thermos/config/schema_base.py
https://reviews.apache.org/r/28920/#comment107932

I don't think name is used anywhere, so we should just remove it.


- Jay Buffington


On Dec. 11, 2014, 6:16 a.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Dec. 11, 2014, 6:16 a.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample script is in 
 examples/jobs/docker, as well as an example aurora file.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 The thermos root path defaults to /var/run/thermos, however if a different 
 path is used, it must be passed to the scheduler via 
 `--thermos_observer_root=some path`
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 5665c69cd7b49c3fd7345074c9f16a3b224496ab 
   api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 
 2c449a491bc5a8ac858ea6487e4cef0591f36f66 
   examples/jobs/docker/Dockerfile PRE-CREATION 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/jobs/docker/hello_docker.py PRE-CREATION 
   examples/jobs/docker/thermos_executor.sh PRE-CREATION 
   examples/vagrant/aurorabuild.sh 69983d0140b76c6869cd04e55d760f3e3a1e4262 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 ead9d28100673440168a32d114ecaa15874978a6 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
 9df9b4b79c0c7d29c5088409bf15c0d32a621df0 
   src/main/python/apache/aurora/executor/common/sandbox.py 
 f47a32b3fefb4a89940b1ddc473b8316ac00df12 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 5e4bd65537d186459003c0b9434f1b769e04f448 
   src/main/python/apache/thermos/bin/thermos_runner.py 
 647de2771f301b17de33d8b45198c211d2e84367 
   src/main/python/apache/thermos/config/schema_base.py

Re: Review Request 26856: Adding storage configuration summary.

2014-10-17 Thread Jay Buffington

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



docs/storage_config.md
https://reviews.apache.org/r/26856/#comment97601

A summary here would be good to help give an overview of what the point of 
this document is and who the intended audience is.



docs/storage_config.md
https://reviews.apache.org/r/26856/#comment97599

It is not clear what application these flags are for.  The scheduler 
binary, right?  Maybe link to a page that enumerates all flags.



docs/storage_config.md
https://reviews.apache.org/r/26856/#comment97600

This is a knit, but I would s/this document/the replicated log 
configuration document/;


- Jay Buffington


On Oct. 17, 2014, 3:16 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26856/
 ---
 
 (Updated Oct. 17, 2014, 3:16 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-839
 https://issues.apache.org/jira/browse/AURORA-839
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding config summary for storage.
 
 
 Diffs
 -
 
   docs/storage_config.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26856/diff/
 
 
 Testing
 ---
 
 https://github.com/maxim111333/incubator-aurora/blob/storage_config_summary/docs/storage_config.md
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26822: Fix table of contents in configuration-tutorial.md

2014-10-17 Thread Jay Buffington

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

Ship it!


Thanks!  That always bugged me!

- Jay Buffington


On Oct. 16, 2014, 8:36 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26822/
 ---
 
 (Updated Oct. 16, 2014, 8:36 p.m.)
 
 
 Review request for Aurora, Jay Buffington and Kevin Sweeney.
 
 
 Bugs: AURORA-844
 https://issues.apache.org/jira/browse/AURORA-844
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix table of contents in configuration-tutorial.md
 
 
 Diffs
 -
 
   docs/configuration-tutorial.md 67998e9dab6ac429d96d7c0d2df959336b767f32 
 
 Diff: https://reviews.apache.org/r/26822/diff/
 
 
 Testing
 ---
 
 Rendered here: 
 https://github.com/wfarner/incubator-aurora/blob/wfarner/config_toc/docs/configuration-tutorial.md
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26834: Add client cluster configuration docs.

2014-10-17 Thread Jay Buffington

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



docs/client-cluster-configuration.md
https://reviews.apache.org/r/26834/#comment97605

I this true?  It was my understanding that the only use for this field is 
to format URLs that are  output by the client. 

Instead of using the hostname of the leader it will use this hostname to 
prefix urls.

Usually this would be the url of your VIP in a loadbalancer or a roundrobin 
DNS name.


- Jay Buffington


On Oct. 17, 2014, 12:37 a.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26834/
 ---
 
 (Updated Oct. 17, 2014, 12:37 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-846
 https://issues.apache.org/jira/browse/AURORA-846
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add client cluster configuration docs.
 
 
 Diffs
 -
 
   docs/client-cluster-configuration.md PRE-CREATION 
   docs/client-commands.md f61fc661d77345950d71bd3606dbe3d1488e9e5a 
   docs/developing-aurora-client.md e1b2ccd7504f983169118a288721894184d67c97 
 
 Diff: https://reviews.apache.org/r/26834/diff/
 
 
 Testing
 ---
 
 Rendered here: 
 https://github.com/jcohen/incubator-aurora/blob/jcohen/docs/clusters.json/docs/client-cluster-configuration.md
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 26845: Adding Aurora scheduler storage doc.

2014-10-17 Thread Jay Buffington

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

Ship it!


Sorry for the late review after it already merged.  This doc is great and I 
learned a lot from it.  There's nothing wrong with this doc, but there are a 
few places I think you could expand on things.


docs/storage.md
https://reviews.apache.org/r/26845/#comment97613

I'd like the see a paragraphic justifying avoiding the more traditional 
approaching of writing this data to a database like mysql or cassandra.  I 
think the justification is to remove deployment dependencies?



docs/storage.md
https://reviews.apache.org/r/26845/#comment97610

I haven't really looked at the update features, yet.  Is this live and 
ready to go?  If not maybe defer referencing this until the updater work is 
shipped?



docs/storage.md
https://reviews.apache.org/r/26845/#comment97607

This is pretty much the complete list of what is stored in the replicated 
log, right?  s/For Example/Aurora stores the follow data/



docs/storage.md
https://reviews.apache.org/r/26845/#comment97612

Link to your doc that describes how to config and recover backups.  When I 
read this I immediatly want to know how often the snapshots are taken and what 
negative impact there is from taking snapshots too frequently.



docs/storage.md
https://reviews.apache.org/r/26845/#comment97609

The ellipses serve no purpose. I'd remove them, but this is a minor knit.


- Jay Buffington


On Oct. 17, 2014, 12:21 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26845/
 ---
 
 (Updated Oct. 17, 2014, 12:21 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-839
 https://issues.apache.org/jira/browse/AURORA-839
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Created a high level storage architecture write up.
 
 
 Diffs
 -
 
   docs/images/storage_hierarchy.png PRE-CREATION 
   docs/storage.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26845/diff/
 
 
 Testing
 ---
 
 https://github.com/maxim111333/incubator-aurora/blob/storage_doc/docs/storage.md
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26836: Cron documentation

2014-10-17 Thread Jay Buffington

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



docs/cron-jobs.md
https://reviews.apache.org/r/26836/#comment97615

I'd add a link here like this:

The full specification for cron_schedule is [described below(however you 
anchor link to ## Technical Note About Syntax)



docs/cron-jobs.md
https://reviews.apache.org/r/26836/#comment97614

s/indepdendently/independently/


- Jay Buffington


On Oct. 17, 2014, 12:43 a.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26836/
 ---
 
 (Updated Oct. 17, 2014, 12:43 a.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-440
 https://issues.apache.org/jira/browse/AURORA-440
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Cron documentation.
 
 
 Diffs
 -
 
   docs/configuration-reference.md 83d0d104554753dab939bf8d25b83e9adb00758a 
   docs/cron-jobs.md PRE-CREATION 
   examples/jobs/cron_hello_world.aurora PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26836/diff/
 
 
 Testing
 ---
 
 Pushed to 
 https://github.com/kevints/incubator-aurora/blob/kts/cron-docs/docs/cron-jobs.md
 
 
 Thanks,
 
 Kevin Sweeney
 




Re: Review Request 26834: Add client cluster configuration docs.

2014-10-16 Thread Jay Buffington


 On Oct. 16, 2014, 11:03 p.m., Joshua Cohen wrote:
  docs/client-cluster-configuration.md, line 34
  https://reviews.apache.org/r/26834/diff/1/?file=723472#file723472line34
 
  I think an argument could be made for removing this configuration 
  entirely and just hardcoding this into the client. I don't think there's a 
  use case where we'd want these commands to talk to any run other than the 
  latest run (correct me if I'm wrong), however having this here does protect 
  us against having to redeploy the client in the event that mesos changes 
  its path structure.
 
 Kevin Sweeney wrote:
 I'd be +1 to dropping this parameter entirely, either in this patch or a 
 followup

At a bare minimum make latest the default and make this optional.


- Jay


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


On Oct. 16, 2014, 10:56 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26834/
 ---
 
 (Updated Oct. 16, 2014, 10:56 p.m.)
 
 
 Review request for Aurora, Maxim Khutornenko and Bill Farner.
 
 
 Bugs: AURORA-846
 https://issues.apache.org/jira/browse/AURORA-846
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add client cluster configuration docs.
 
 
 Diffs
 -
 
   docs/client-cluster-configuration.md PRE-CREATION 
   docs/client-commands.md f61fc661d77345950d71bd3606dbe3d1488e9e5a 
 
 Diff: https://reviews.apache.org/r/26834/diff/
 
 
 Testing
 ---
 
 Rendered here: 
 https://github.com/jcohen/incubator-aurora/blob/jcohen/docs/clusters.json/docs/client-cluster-configuration.md
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 26244: Add a doc about dedicated hosts.

2014-10-01 Thread Jay Buffington

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



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/26244/#comment95556

Before the example I think you should have a more formal definition of the 
value for the dedicated attribute.  

I think what you're saying here is that the format is $role/$job_name.  Do 
I *have* to include job name?  What if I want to dedicate a slave to a role 
rather than a role + job.

I assume environment cannot go into the value for dedicated.


Thanks so much for writing this!

- Jay Buffington


On Oct. 2, 2014, 12:09 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26244/
 ---
 
 (Updated Oct. 2, 2014, 12:09 a.m.)
 
 
 Review request for Aurora and Kevin Sweeney.
 
 
 Bugs: AURORA-703
 https://issues.apache.org/jira/browse/AURORA-703
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add a doc about dedicated hosts.
 
 
 Diffs
 -
 
   docs/deploying-aurora-scheduler.md 93ff746038df14f2abeea85acf48d02b217af522 
 
 Diff: https://reviews.apache.org/r/26244/diff/
 
 
 Testing
 ---
 
 Rendered here: 
 https://github.com/wfarner/incubator-aurora/blob/wfarner/doc_dedicated_machines/docs/deploying-aurora-scheduler.md
 
 
 Thanks,
 
 Bill Farner