On March 13, 2016, 6:04 a.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.
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. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44745/#review123314 ----------------------------------------------------------- On March 12, 2016, 7:48 p.m., John Sirois wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44745/ > ----------------------------------------------------------- > > (Updated March 12, 2016, 7:48 p.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 > >
