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="===============0552683983360542395=="
MIME-Version: 1.0
Subject: Re: Review Request 28920: Add support for docker containers to aurora
From: "Steve Niemitz" <steve@tellapart.com>
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: Mon, 12 Jan 2015 16:11:15 -0000
Message-ID: <20150112161115.23997.40172@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Steve Niemitz" <noreply@reviews.apache.org>
References: <20150108184840.26561.35271@reviews.apache.org>
In-Reply-To: <20150108184840.26561.35271@reviews.apache.org>
Reply-To: "Steve Niemitz" <steve@tellapart.com>

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


> On Jan. 8, 2015, 6:48 p.m., Bill Farner wrote:
> > docs/deploying-aurora-scheduler.md, line 155
> > <https://reviews.apache.org/r/28920/diff/7/?file=808126#file808126line155>
> >
> >     From reading the code, it appears this additional path is needed for them both to be available in the container.  If so, would it be easier to just accept an arbitrary number of additional assets to copy into the sandbox?  I would find that more generalized, and easier to understand.
> >     
> >     If you go with the above, i _think_ you can also safely nuke the extra args plumbing.
> 
> Steve Niemitz wrote:
>     I think we'll still need extra args (its really useful right now to pass ZK config in), but I like the idea of an arbitrary set of resources.  The only trouble here would be figuring out which is the one to run and getting the symlinks right.  Let's talk about this more.
> 
> Bill Farner wrote:
>     Wouldn't the extra args just be solved with the shim script?  I tell the scheduler to copy `my_executor.sh` and `executor.pex` into the container, and `my_executor.sh` contains the extra args.  I like this as a generalized solution to augmenting default executor behavior, since it avoids feature creep on our side just to save people the shim script.
> 
> Steve Niemitz wrote:
>     Correct, it would.  My goal with this (which I'm happy to discuss more) was to make the wrapper script unnecessary in normal cases.  For us we just need to pass in the ZK config for the announcer, and I feel like this is the typical case.  Let me simmer on this one for a little bit.
> 
> Kevin Sweeney wrote:
>     I'd like to entirely avoid the need for a shim script by configuring the scheduler. The rationale being I would like to one day be able to host executor builds on a public artifact store (that a user doesn't necessarily have write access to) with all aurora-specific configuration happening on the scheduler.
>     
>     In that world the slaves would only need to know about the master, and site-specific executor settings could be configured on the scheduler like so:
>     ```
>     -thermos_executor_path=https://some.public.url/thermos-0.7.0.pex
>     -thermos_executor_flags='--announcer-enable --announcer-ensemble=zk://... ...'
>     ```
> 
> Bill Farner wrote:
>     > The rationale being I would like to one day be able to host executor builds on a public artifact store (that a user doesn't necessarily have write access to)
>     
>     Is this not true today?  It's not clear to me how a shim script implies write access.  On the other hand, i find a host-local shim script useful for things like controlled deployment of new arguments.
> 
> Bill Farner wrote:
>     Circling back from discussion in IRC - we will proceed with two additioanl command line arguments to the scheduler: one that specifies arguments to the executor, and one that lists additional executor resources required by the executor.  Please reply if you (anyone) disagree.

Just to make sure we're all on the same page, does that mean running with a wrapper looks like this?

   -thermos_executor_path=/some/path/to/wrapper.sh
   -thermos_executor_resources=/some/other/path/to/thermos_executor.pex
  
And running without looks like this:
   -thermos_executor_path=/some/other/path/to/thermos_executor.pex
   (no -thermos_executor_resources)

If so I'm on board and will have a change set coming soon.  One thing that comes from this however is now each sandbox will have a ~50meg pex file (the executor) copied into it.  This probably isn't a huge deal, but I'd just like to point it out.


- Steve


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


On Jan. 9, 2015, 7:34 p.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2015, 7:34 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 
>   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
>   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
>   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
>   examples/vagrant/aurorabuild.sh b7ea41719a8f41bb23d0254e732926d89399c77c 
>   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 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/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/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
> 
>


--===============0552683983360542395==--