Re: Review Request 44770: Create scheduler-configuration.md

[Aurora ReviewBot]

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


Ship it!




Master (de128a1) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On March 13, 2016, 11:21 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44770/
> ---
> 
> (Updated March 13, 2016, 11:21 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Having a simple dump of the all scheduler configuration options makes it much 
> easier to discover or find them via a public search engine.
> 
> Ideally we would have a way to update this automatically, but anything is 
> better than not having them listed at all.
> 
> 
> Diffs
> -
> 
>   docs/README.md 78f062abb86faf9ac0347c02828d78a544c74198 
>   docs/deploying-aurora-scheduler.md 03ee360a398821dcc8ca74001bfac478241ad5d8 
>   docs/scheduler-configuration.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44770/diff/
> 
> 
> Testing
> ---
> 
> Rendered version online at 
> https://github.com/StephanErb/aurora/blob/cmdopts/docs/scheduler-configuration.md
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Bill Farner]

On March 13, 2016, 5: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.

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.


- Bill


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


On March 12, 2016, 6: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, 6: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='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/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: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44745: Allow for a pure docker executor.

[Stephan Erb]

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




src/main/python/apache/aurora/config/__init__.py (line 142)


Unnecessary space



src/main/python/apache/aurora/config/thrift.py (line 249)


How about using dependency injection for the `ExecutorConfig`? Having such 
an `if` in a pure translation/serialization function feels like we are 
implementing logic at a wrong layer.



src/main/python/apache/thermos/config/schema_base.py (line 96)


What will happen to the Task name if the process list is empty? Runtime 
exception in Thermos?


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.

- Stephan Erb


On March 13, 2016, 3: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, 3: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='),
>   Parameter(name = 'expose', value = ''),
>   Parameter(name = 'publish', value = ':/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: and get `Hello!` back.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 44532: Allow overriding hostname before announcing

[Stephan Erb]

> On March 10, 2016, 11:04 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line 95
> > 
> >
> > Specifying the command here feels kind of weird. All other options  we 
> > have take the value directly.
> 
> Kunal Thakar wrote:
> Executor arguments are set in stone after starting the scheduler as they 
> are passed to the scheduler through -thermos_executor_flags, so we have to do 
> it this way.

Another way to do it would be if you use a wrapper script to launch your 
Thermos executor. If necessary, you can copy this into the sandbox using the 
`-thermos_executor_resources` flag. 

Personally, I would perfer such a solution:

* It can be used for all options.
* Thermos does not need to handle the case if the command fails. It is up to 
the operator writing the wrapper script to handle that case.


- Stephan


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


On March 8, 2016, 11:33 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44532/
> ---
> 
> (Updated March 8, 2016, 11:33 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Allow overriding hostname before announcing
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> f82858c528808d2a9e77bb56f16e897cfb5bbe73 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 34e36e0a59093468a8934f58bacb68512949347c 
> 
> Diff: https://reviews.apache.org/r/44532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>