aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Khutornenko <ma...@apache.org>
Subject Re: Review Request 44745: Allow for a pure docker executor.
Date Tue, 15 Mar 2016 16:47:35 GMT


On March 13, 2016, 12:04 p.m., John Sirois wrote:
> > While your patch is rather easy, I am not sure it is the best way to move forward.
It feels like it is crossing streams with https://issues.apache.org/jira/browse/AURORA-1288.
Putting some thought into this might be helpful in the long run.
> 
> Bill Farner wrote:
>     FWIW i don't think it complicates or even diverges from that ticket.  In my opinion
it's yet to be seen whether it's feasible to use the same client for a custom executor (at
least, without a decent amount of modularization work).  At the very least, that effort has
lost momentum and we shouldn't block progress for it.
> 
> Stephan Erb wrote:
>     I mostly brought it up because the ticket also repeatedly mentions the default Mesos
command executor. Supporting this one does not sound to different from supporting Docker without
Thermos. It would also need similar logic at the UI layer to allow use the Mesos sanbox UI
instead of the Thermos one.
>     
>     I agree that we should not block progress here. I justed wanted to make sure we are
not rushing things (i.e., there isn't even a jira ticket right now).
> 
> Maxim Khutornenko wrote:
>     +1 to Stephan's concerns. The schema changes in this patch don't necessarily convey
enough meaning to paint a clear picture of where this effort leads us. FWICT, nothing in the
Task aside from resources is applicable to the Docker case and it feels quite hacky to onboard
a new executor case this way.
>     
>     > In my opinion it's yet to be seen whether it's feasible to use the same client
for a custom executor (at least, without a decent amount of modularization work).
>     
>     Bill, would you mind clarifying what this means? Are you expecting this to be a purely
experimental (POC) effort or is there a solid production quality future here? If it's the
former, would it be more appropriate to have this effort baked in a private branch to avoid
possibly unnecessary code churn and review cycles?
> 
> Bill Farner wrote:
>     | it feels quite hacky to onboard a new executor case this way
>     
>     Suggestions solicited!  Just please don't forget that the intent is to offer real,
immediate value - the docker support in Aurora today is quite crippled, and this will address
the biggest shortcomings (entrypoints, and zero required deps in images).
>     
>     | FWICT, nothing in the Task aside from resources is applicable to the Docker case
>     
>     This is a good point.  Perhaps we should create a separate struct and field in `Job`
for this case?
>     
>     | Bill, would you mind clarifying what this means?
>     
>     What i mean is that the client, DSL, and executor all have relatively high coupling.
 Adding custom executor support in the client will require non-trivial effort to break that
coupling.  I would like to avoid blocking this feature on that goal.
>     
>     | Are you expecting this to be a purely experimental (POC) effort or is there a solid
production quality future here?
>     
>     That is very much dependent on the underlying support in mesos.  Today, i see it
as the best support for docker containers in mesos.  It's been available for some time, and
the work here is entirely plumbing to enable it in Aurora.
>     
>     | would it be more appropriate to have this effort baked in a private branch to avoid
possibly unnecessary code churn and review cycles?
>     
>     I don't foresee enough churn to warrant that.
> 
> John Sirois wrote:
>     Noting that I'm backing off this change until sentiment settles one way or the other.
 If it settles in-favor I'll address both Stephan & Joshua's feedback at that point.
> 
> Stephan Erb wrote:
>     I have to backoff out of the discussion here, as I don't have the necessary cycles
to participate. A couple of closing notes from my side:
>     
>     * I agree with Maxim that giving an empty process list a special meaning feels kind
of like a hack.
>     * I probably wouldn't have complained about this if it was that way from the beginning...
>     * Docker support is still considered experimental, so no decision is cast in stone.
We can change stuff without to much hassle.
>     * It is great that you are improving the current docker support (even though I am
a fanboy of the upcoming unified container support :-)

Thanks for explaning Bill. I am fine continuing this effort given the above.

> This is a good point.  Perhaps we should create a separate struct and field in Job for
this case?

I don't have bandwidth to think about the alternatives at the moment but your suggestion about
plugging it higher in the chain (e.g. Job struct) sounds logical.


- Maxim


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


On March 13, 2016, 2:48 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44745/
> -----------------------------------------------------------
> 
> (Updated March 13, 2016, 2:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows for a job config with no processes defined IFF there is also
> a Docker container defined.  In this case it is assumed that the process
> to run is the Docker container's ENTRYPOINT via the Mesos Docker
> containerizer.
> 
>  src/main/python/apache/aurora/config/__init__.py    | 26 +++++++++++++++++++++++---
>  src/main/python/apache/aurora/config/thrift.py      |  9 +++++----
>  src/test/python/apache/aurora/client/test_config.py | 41 +++++++++++++++++++++++++++++++++++------
>  src/test/python/apache/aurora/config/test_thrift.py |  5 +++++
>  4 files changed, 68 insertions(+), 13 deletions(-)
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py 65923be1cb8b88139b8eab0ac5b75428972d3cb1

>   src/main/python/apache/aurora/config/thrift.py be0cd68674a71bd4baadf276f40a4bc0223ce4be

>   src/main/python/apache/thermos/config/schema_base.py a6768e67189b0560afef844d6b269bed8ada5f2f

>   src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a

>   src/test/python/apache/aurora/config/test_thrift.py 88292d3c4423c0555088a0adaee3c0e62ed0567e

> 
> Diff: https://reviews.apache.org/r/44745/diff/
> 
> 
> Testing
> -------
> 
> Locally green `./build-support/jenkins/build.sh`
> 
> I also patched in https://reviews.apache.org/r/44685/ which this change
> depends on and was able to run scheduler with
> `-allow_docker_parameters -require_docker_use_executor` and successfully
> run this job:
> ```
> import getpass
> 
> jobs=[
>   Service(
>     cluster = 'devcluster',
>     role = getpass.getuser(),
>     environment = 'test',
>     name = 'http_example_docker_executor',
>     contact = '{{role}}@localhost',
>     instances = 1,
>     task = Task(
>       name = 'http_docker_example',
>       resources = Resources(cpu=0.4, ram=32*MB, disk=64*MB),
>       processes = []
>     ),
>     container = Container(
>       docker = Docker(
>         image = 'http_example',
>         parameters = [
>           Parameter(name = 'env', value = 'HTTP_PORT=8888'),
>           Parameter(name = 'expose', value = '8888'),
>           Parameter(name = 'publish', value = '8888:8888/tcp'),
>         ],
>       ),
>     ),
>   )
> ]
> ```
> 
> Using the image created with
> `docker build -t http_example src/test/sh/org/apache/aurora/e2e` from:
> ```
> FROM python:2.7
> 
> # mesos.native requires libcurl-nss to initialize MesosExecutorDriver
> RUN apt-get update && apt-get -y install libcurl4-nss-dev
> 
> COPY http_example.py /tmp/
> ENTRYPOINT python /tmp/http_example.py $HTTP_PORT
> ```
> 
> I could connect to http://aurora.local:8888 and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message