Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============2954623942740407430=="
MIME-Version: 1.0
Subject: Re: Review Request 28920: Add support for docker containers to aurora
From: "Kevin Sweeney" <kevints@apache.org>
To: "Bill Farner" <wfarner@apache.org>, "Jay Buffington" <me@jaybuff.com>,
 "Kevin Sweeney" <kevints@apache.org>
Cc: "Benjamin Staffin" <ben@folsomlabs.com>,
 "Darrin Eden" <darrin.eden@gmail.com>,
 "Aurora" <reviews@aurora.incubator.apache.org>,
 "Steve Niemitz" <steve@tellapart.com>
Date: Thu, 22 Jan 2015 23:39:11 -0000
Message-ID: <20150122233911.17164.23447@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Kevin Sweeney" <noreply@reviews.apache.org>
References: <20150122023500.17164.68430@reviews.apache.org>
In-Reply-To: <20150122023500.17164.68430@reviews.apache.org>
Reply-To: "Kevin Sweeney" <kevints@apache.org>

--===============2954623942740407430==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Jan. 21, 2015, 6:35 p.m., Kevin Sweeney wrote:
> > examples/vagrant/provision-dev-cluster.sh, line 17
> > <https://reviews.apache.org/r/28920/diff/18/?file=823204#file823204line17>
> >
> >     nit: sh -c indirection is unnecessary here.
> 
> Steve Niemitz wrote:
>     Eh, this is just copied from the [docker install docs](https://docs.docker.com/installation/ubuntulinux/#ubuntu-precise-1204-lts-64-bit).

The `sh -c` indirection is necessary there because of the use of `sudo`. In a script that's already running as root it's unnecesary.


> On Jan. 21, 2015, 6:35 p.m., Kevin Sweeney wrote:
> > docs/deploying-aurora-scheduler.md, line 163
> > <https://reviews.apache.org/r/28920/diff/18/?file=823201#file823201line163>
> >
> >     Philosophical question: if there's already a hard requirement that the container have Python 2.7 why not require that the executor be baked in as well? Maybe it's worth calling out as a TODO, but you don't have to answer it now.
> 
> Steve Niemitz wrote:
>     I think baking the executor into docker images is a recipe for disaster.  Any time you upgraded aurora you'd need to then go update all containers with the new executor.  Also, I don't like the idea of having to build specific aurora-isms into docker containers (I don't even really like requiring python, but that's unavoidable).
> 
> Bill Farner wrote:
>     I'm with Steve here.  Forcing this seems overly restrictive.  However, i would like to support the mode of operation you describe, Kevin.

Just seems weird to put partial requirements on the execution environment of the container we can run, which somewhat defeats the purpose of containers.

Filed https://issues.apache.org/jira/browse/AURORA-1051 to explore removing this restriction.


> On Jan. 21, 2015, 6:35 p.m., Kevin Sweeney wrote:
> > docs/deploying-aurora-scheduler.md, line 158
> > <https://reviews.apache.org/r/28920/diff/18/?file=823201#file823201line158>
> >
> >     Latest version doesn't require a wrapper script, update docs to reflect?
> 
> Steve Niemitz wrote:
>     The previous sentence here is "If using a wrapper script".

Gotcha, my bad.


- Kevin


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


On Jan. 15, 2015, 4:08 p.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 4:08 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 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/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>


--===============2954623942740407430==--