You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Dmitriy Shirchenko <ca...@gmail.com> on 2016/03/08 02:42:54 UTC
Re: Review Request 44486: Exposing ports to shell health checkers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 8, 2016, 1:42 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Summary (updated)
-----------------
Exposing ports to shell health checkers
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description (updated)
-------
Exposing ports to shell health checkers
Diffs
-----
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122537
-----------------------------------------------------------
Fix it, then Ship it!
LGTM - thanks for the doc additions.
Zameer will take this review home.
NEWS (line 17)
<https://reviews.apache.org/r/44486/#comment184587>
This belongs in the `New/updated` section above and it's probably worth linking the new docs on github. The 0.12.0 entry below starting with `- Added support for jobs to specify arbitrary ZooKeeper paths...` is a good example.
docs/configuration-reference.md (line 453)
<https://reviews.apache.org/r/44486/#comment184588>
s/look/looks/ and perhaps language more closely aligned with environment variables - perhaps something like "... will have the `HTTP_PORT` variable exported in its environment."
- John Sirois
On March 7, 2016, 7:10 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 7:10 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Joshua Cohen <jc...@apache.org>.
> On March 8, 2016, 7:25 p.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
I think this is an excellent point, good catch Zameer!
Dmitriy, is there any reason why this approach won't work for you guys?
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 6:32 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 6:32 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Zameer Manji <zm...@apache.org>.
> On March 8, 2016, 11:25 a.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> Joshua Cohen wrote:
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
>
> Dmitriy Shirchenko wrote:
> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>
> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
Using the DSL doesn't mean arguments, you can do something like this:
'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 10:32 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 10:32 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
> On March 8, 2016, 7:25 p.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> Joshua Cohen wrote:
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
>
> Dmitriy Shirchenko wrote:
> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>
> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
>
> Zameer Manji wrote:
> Using the DSL doesn't mean arguments, you can do something like this:
>
> 'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
Here's the code that runs the command:
```
cmd = shlex.split(self.cmd)
try:
subprocess.check_call(cmd, timeout=self.timeout_secs)
```
so if self.cmd is 'HTTP_PORT=123 RPC_PORT=234 /usr/bin/health_checker' aws you are suggesting, how would this work? `check_call` doesn't pass through environment variables w/out shell=True AFAIK. shell=True has security concerns so it's disabled by default.
- Dmitriy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 6:32 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 6:32 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
> On March 8, 2016, 7:25 p.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> Joshua Cohen wrote:
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
- Dmitriy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 6:32 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 6:32 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Bill Farner <wf...@apache.org>.
> On March 8, 2016, 11:25 a.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> Joshua Cohen wrote:
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
>
> Dmitriy Shirchenko wrote:
> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>
> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
>
> Zameer Manji wrote:
> Using the DSL doesn't mean arguments, you can do something like this:
>
> 'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
>
> Dmitriy Shirchenko wrote:
> Here's the code that runs the command:
> ```
> cmd = shlex.split(self.cmd)
> try:
> subprocess.check_call(cmd, timeout=self.timeout_secs)
> ```
>
> so if self.cmd is 'HTTP_PORT=123 RPC_PORT=234 /usr/bin/health_checker' aws you are suggesting, how would this work? `check_call` doesn't pass through environment variables w/out shell=True AFAIK. shell=True has security concerns so it's disabled by default.
>
> Zameer Manji wrote:
> Can you elaborate with with the security concerns?
>
> Currently all of the other processes launched by thermos have the cmd string passed to the shell which is very convinent. I think doing that here would be useful as well. Infact, we could just replace the subprocess work here by just re-using the existing thermos code we have which will pass the cmd string to bash, interpolate variables and setuid, setguid so the health check process doesn't run a root.
I'm glad you raised this, Zameer. For some reason in my head i actually convinced myself this is how it worked. I can see how both environment variables and mustache-style templating could be useful, and agree that we should offer templating for consistency. I assume this is trivial to support and include in this patch.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 10:32 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 10:32 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Zameer Manji <zm...@apache.org>.
Currently the health checker is just launched by the executor blindly. This
proposal now plumbs information about the task to the subprocess. We
already have a mechanism to do this, it's the DSL provided to users when
they compose their processes. In that case we compose a command line with
variables like './foo --port={{thermos.ports[http]}}'. When this string is
passed to thermos (the parts of the executor that actually launch the
process), the variables are interpolated and the command string is passed
to $SHELL which then runs the process.
This proposal diverges from the existing setup because now ports are passed
to the subprocess via a completely different method. This proposal is also
limited because only ports are passed to the subprocess. Why not other
information available about the task such as the cluster, role, or
hostname? We can currently pass that information to processes via the DSL
as well by having cmdlines like './process --hostname={{mesos.hostname}}'.
I think removing this inconsistency is good, because the DSL becomes easier
to use and there is less of a support burden when educating users about the
DSL.
To implement this I see at least two (if not more) approaches:
- The complex method involves using the code from thermos (the code that
runs a process, interpolates variables, etc) into the shell health checker
directly. It's a bit complex but it is doable and it has the most code
reuse. It's different than using the subprocess32 module, because instead
of spawning a subprocess, thermos will fork off a 'runner' process which
then creates the desired process as a subprocess and the runner
communicates state back to the caller via writing the process state to
checkpoint files.
- The alternative approach involves setting shell=True, and using
the pystachio library (the implementation of the interpolation) to fill in
the variables and then passing that string to subprocess32. This has a two
benefits: it is much simpler and this is already done elsewhere in the
code. The only drawback is that we have to do shell=True to prevent the
argument issue you were describing, but it was objected upon for security
reasons and it is a breaking change (although it seems Uber is the only
user so we can skip over that).
I think the latter approach is preferable and I don't see it being a lot of
extra code. I think we can ignore the security implications from shell=True
for two reasons:
- The documentation says "[...], unsanitized input from an untrusted
source, [...]" which is not true here. Aurora assumes that the cmd strings
provided in the DSL comes from the user which is authenticated by the
scheduler when submitting the task.
- The other process strings consumed by the DSL are passed to thermos
which currently passes it to $SHELL, so we are already open to this kind of
issue.
To implement the latter approach you can take a look at
`DistributedCommandRunner` in
`src/main/python/apache/aurora/client/api/command_runner.py` which supports
running a command in the context of an already running task and does
variable interpolation.
Specifically you can see this code:
````
@classmethod
def substitute(cls, command, task, cluster, **kw):
prefix_command = 'cd %s;' % cls.thermos_sandbox(cluster, **kw)
thermos_namespace = ThermosContext(
task_id=task.assignedTask.taskId,
ports=task.assignedTask.assignedPorts)
mesos_namespace = MesosContext(instance=task.assignedTask.instanceId)
command = String(prefix_command + command) % Environment(
thermos=thermos_namespace,
mesos=mesos_namespace)
return command.get()
````
This code takes a command string, the task object and other parameters and
uses the pystachio library to interpolate the values into the string.
I don't think constructing the ThermosContext/MesosContext objects is too
hard in the shell health checker and the interpolation is only two extra
lines.
I hope this clears up what I think should be done to this code.
On Tue, Mar 8, 2016 at 3:12 PM, Dmitriy Shirchenko <ca...@gmail.com>
wrote:
> https://docs.python.org/2/library/subprocess.html#frequently-used-arguments
>
> Ok, sorry, so what exactly are you proposing I do instead of simply
> passing in environment variables?
>
> What themos code? What do we replace subprocess with?
>
> I'm new to this code base so I'm still figuring things out so pardon my
> ignorance.
>
> Do you have strong concerns about this approach or just want it to be
> perfect?
>
> On Tue, Mar 8, 2016, 12:04 PM Zameer Manji <zm...@apache.org> wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/44486/
>>
>> On March 8th, 2016, 11:25 a.m. PST, *Zameer Manji* wrote:
>>
>> The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
>>
>> Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
>>
>> For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>>
>> On March 8th, 2016, 11:29 a.m. PST, *Joshua Cohen* wrote:
>>
>> I think this is an excellent point, good catch Zameer!
>>
>> Dmitriy, is there any reason why this approach won't work for you guys?
>>
>> On March 8th, 2016, 11:39 a.m. PST, *Dmitriy Shirchenko* wrote:
>>
>> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>>
>> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
>>
>> On March 8th, 2016, 11:42 a.m. PST, *Zameer Manji* wrote:
>>
>> Using the DSL doesn't mean arguments, you can do something like this:
>>
>> 'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
>>
>> On March 8th, 2016, 11:49 a.m. PST, *Dmitriy Shirchenko* wrote:
>>
>> Here's the code that runs the command:
>>
>> cmd = shlex.split(self.cmd)
>> try:
>> subprocess.check_call(cmd, timeout=self.timeout_secs)
>>
>> so if self.cmd is 'HTTP_PORT=123 RPC_PORT=234 /usr/bin/health_checker'
>> aws you are suggesting, how would this work? check_call doesn't pass
>> through environment variables w/out shell=True AFAIK. shell=True has
>> security concerns so it's disabled by default.
>>
>> Can you elaborate with with the security concerns?
>>
>> Currently all of the other processes launched by thermos have the cmd string passed to the shell which is very convinent. I think doing that here would be useful as well. Infact, we could just replace the subprocess work here by just re-using the existing thermos code we have which will pass the cmd string to bash, interpolate variables and setuid, setguid so the health check process doesn't run a root.
>>
>>
>> - Zameer
>>
>> On March 8th, 2016, 10:32 a.m. PST, Dmitriy Shirchenko wrote:
>> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>> By Dmitriy Shirchenko.
>>
>> *Updated March 8, 2016, 10:32 a.m.*
>> *Bugs: * AURORA-1622 <https://issues.apache.org/jira/browse/AURORA-1622>
>> *Repository: * aurora
>> Description
>>
>> Exposing ports to shell health checkers
>>
>> Testing
>>
>> Unit and end to end test.
>>
>> Diffs
>>
>> - NEWS (b84a94550f93691eba0220afedb2bb4d5e00e6bd)
>> - docs/configuration-reference.md
>> (10702ff4e700b6da7bdd7fd036de442be1eba45c)
>> - src/main/python/apache/aurora/common/health_check/shell.py
>> (890bf0c5d50d0022c044a37191a2e3145cc6340f)
>> - src/main/python/apache/aurora/executor/common/health_checker.py
>> (303972778baa04e9d7dd47fb208fe1427e779976)
>> - src/test/python/apache/aurora/common/health_check/test_shell.py
>> (84f717fbf724c11863b4980fd2740dc23fe1404e)
>> - src/test/python/apache/aurora/executor/common/test_health_checker.py
>> (9bebce8f5a26662f58075d7ce881a8bdacb2fe46)
>>
>> View Diff <https://reviews.apache.org/r/44486/diff/>
>>
>
--
Zameer Manji
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
https://docs.python.org/2/library/subprocess.html#frequently-used-arguments
Ok, sorry, so what exactly are you proposing I do instead of simply passing
in environment variables?
What themos code? What do we replace subprocess with?
I'm new to this code base so I'm still figuring things out so pardon my
ignorance.
Do you have strong concerns about this approach or just want it to be
perfect?
On Tue, Mar 8, 2016, 12:04 PM Zameer Manji <zm...@apache.org> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
>
> On March 8th, 2016, 11:25 a.m. PST, *Zameer Manji* wrote:
>
> The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
>
> Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
>
> For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> On March 8th, 2016, 11:29 a.m. PST, *Joshua Cohen* wrote:
>
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
>
> On March 8th, 2016, 11:39 a.m. PST, *Dmitriy Shirchenko* wrote:
>
> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>
> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
>
> On March 8th, 2016, 11:42 a.m. PST, *Zameer Manji* wrote:
>
> Using the DSL doesn't mean arguments, you can do something like this:
>
> 'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
>
> On March 8th, 2016, 11:49 a.m. PST, *Dmitriy Shirchenko* wrote:
>
> Here's the code that runs the command:
>
> cmd = shlex.split(self.cmd)
> try:
> subprocess.check_call(cmd, timeout=self.timeout_secs)
>
> so if self.cmd is 'HTTP_PORT=123 RPC_PORT=234 /usr/bin/health_checker' aws
> you are suggesting, how would this work? check_call doesn't pass through
> environment variables w/out shell=True AFAIK. shell=True has security
> concerns so it's disabled by default.
>
> Can you elaborate with with the security concerns?
>
> Currently all of the other processes launched by thermos have the cmd string passed to the shell which is very convinent. I think doing that here would be useful as well. Infact, we could just replace the subprocess work here by just re-using the existing thermos code we have which will pass the cmd string to bash, interpolate variables and setuid, setguid so the health check process doesn't run a root.
>
>
> - Zameer
>
> On March 8th, 2016, 10:32 a.m. PST, Dmitriy Shirchenko wrote:
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
> By Dmitriy Shirchenko.
>
> *Updated March 8, 2016, 10:32 a.m.*
> *Bugs: * AURORA-1622 <https://issues.apache.org/jira/browse/AURORA-1622>
> *Repository: * aurora
> Description
>
> Exposing ports to shell health checkers
>
> Testing
>
> Unit and end to end test.
>
> Diffs
>
> - NEWS (b84a94550f93691eba0220afedb2bb4d5e00e6bd)
> - docs/configuration-reference.md
> (10702ff4e700b6da7bdd7fd036de442be1eba45c)
> - src/main/python/apache/aurora/common/health_check/shell.py
> (890bf0c5d50d0022c044a37191a2e3145cc6340f)
> - src/main/python/apache/aurora/executor/common/health_checker.py
> (303972778baa04e9d7dd47fb208fe1427e779976)
> - src/test/python/apache/aurora/common/health_check/test_shell.py
> (84f717fbf724c11863b4980fd2740dc23fe1404e)
> - src/test/python/apache/aurora/executor/common/test_health_checker.py
> (9bebce8f5a26662f58075d7ce881a8bdacb2fe46)
>
> View Diff <https://reviews.apache.org/r/44486/diff/>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Zameer Manji <zm...@apache.org>.
> On March 8, 2016, 11:25 a.m., Zameer Manji wrote:
> > The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
> >
> > Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
> >
> > For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
>
> Joshua Cohen wrote:
> I think this is an excellent point, good catch Zameer!
>
> Dmitriy, is there any reason why this approach won't work for you guys?
>
> Dmitriy Shirchenko wrote:
> Yea, I tried using that approach at first. But we need to allocate 10 ports (mix of HTTP and another RPC protocol(s)) for some services, and our existing health check scripts at the moment are just simple bash scripts. Dealing with 10 arguments will become difficult (imagine writing one for that case), especially since order will matter and the owner will need to keep track of order in which they are passed in (eg is it an HTTP one, or some RPC protocol.. oh wait, I thought that was passed in first... dammit I have to read code in how our internal system massages them since we bypass aurora client completely to see how it actually works). Environment variables are just easier to deal with.
>
> Does this make sense? Perhaps I could have explained why this approach in the Summary/Description.
>
> Zameer Manji wrote:
> Using the DSL doesn't mean arguments, you can do something like this:
>
> 'HTTP_PORT={{thermos.ports[http]}} RPC_PORT={{thermos.ports[rpc]}} /usr/bin/health_checker'
>
> Dmitriy Shirchenko wrote:
> Here's the code that runs the command:
> ```
> cmd = shlex.split(self.cmd)
> try:
> subprocess.check_call(cmd, timeout=self.timeout_secs)
> ```
>
> so if self.cmd is 'HTTP_PORT=123 RPC_PORT=234 /usr/bin/health_checker' aws you are suggesting, how would this work? `check_call` doesn't pass through environment variables w/out shell=True AFAIK. shell=True has security concerns so it's disabled by default.
Can you elaborate with with the security concerns?
Currently all of the other processes launched by thermos have the cmd string passed to the shell which is very convinent. I think doing that here would be useful as well. Infact, we could just replace the subprocess work here by just re-using the existing thermos code we have which will pass the cmd string to bash, interpolate variables and setuid, setguid so the health check process doesn't run a root.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
On March 8, 2016, 10:32 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 10:32 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122583
-----------------------------------------------------------
The code for this approach looks fine to me, but I'm not sure if this approach is the way to go.
Why can't the command for the health checker include '{{thermos.ports[http]}}' and we can resolve that value before launching the subprocess? Thats more consistent with the rest of the DSL. Further, using the mustache variables in the command variable would allow the health checker process to have access to all of the same information that task processes have like hostname.
For example the command could be '/usr/bin/health_checker --port-to-check={{thermos.ports[http]}}'.
- Zameer Manji
On March 8, 2016, 10:32 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 10:32 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122681
-----------------------------------------------------------
Master (bab21ed) is red with this patch.
./build-support/jenkins/build.sh
[1m proxy_driver = ProxyDriver()[0m
[1m with temporary_dir() as checkpoint_root:[0m
[1m te = AuroraExecutor([0m
[1m> runner_provider=make_provider(checkpoint_root),[0m
[1m sandbox_provider=DefaultTestSandboxProvider())[0m
src/test/python/apache/aurora/executor/test_thermos_executor.py:580:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in make_provider
[1m pex_location=thermos_runner_path(),[0m
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
build = True
[1m def thermos_runner_path(build=True):[0m
[1m if not build:[0m
[1m return getattr(thermos_runner_path, 'value', None)[0m
[1m [0m
[1m if not hasattr(thermos_runner_path, 'value'):[0m
[1m pex_dir = safe_mkdtemp()[0m
[1m> assert subprocess.call(["./pants", "--pants-distdir=%s" % pex_dir, "binary",[0m
[1m "src/main/python/apache/thermos/runner:thermos_runner"]) == 0[0m
[1m[31mE assert 1 == 0[0m
[1m[31mE + where 1 = <function call at 0x7f628b5be938>(['./pants', '--pants-distdir=/tmp/user/10021/tmprqo_qQ', 'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])[0m
[1m[31mE + where <function call at 0x7f628b5be938> = subprocess.call[0m
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: AssertionError
-------------- Captured stderr call --------------
Traceback (most recent call last):
File "/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.71/bin/pants", line 7, in <module>
from pants.bin.pants_exe import main
ImportError: No module named pants.bin.pants_exe
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 16 failed, 637 passed, 5 skipped, 1 warnings, 8 error in 218.19 seconds [0m
FAILURE
05:50:22 04:32 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 9, 2016, 5:34 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 5:34 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122688
-----------------------------------------------------------
Ship it!
Master (bab21ed) 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 9, 2016, 5:34 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 5:34 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122683
-----------------------------------------------------------
@ReviewBot retry
- Dmitriy Shirchenko
On March 9, 2016, 5:34 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 5:34 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122943
-----------------------------------------------------------
Fix it, then Ship it!
Modulo a missing test, this LGTM.
I will not commit this patch after the missing test is added as I think Bill and John should review the changes again.
src/test/python/apache/aurora/executor/common/test_health_checker.py (line 269)
<https://reviews.apache.org/r/44486/#comment185047>
Can you add a test that checks interpolation is done? Like pass in assignedPorts and then check in `execconfig_data['health_check_config'['health_checker']['shell']['shell_command']` that the interpolation was done?
- Zameer Manji
On March 8, 2016, 9:34 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 9:34 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review123202
-----------------------------------------------------------
Ship it!
Ship It!
- John Sirois
On March 10, 2016, 11:59 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 10, 2016, 11:59 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review123018
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Farner
On March 10, 2016, 10:59 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 10, 2016, 10:59 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122968
-----------------------------------------------------------
Ship it!
Master (31a538f) 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 10, 2016, 6:59 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 10, 2016, 6:59 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 10, 2016, 6:59 p.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing DSL defined variables to shell health checkers
Diffs (updated)
-----
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 9, 2016, 5:34 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing DSL defined variables to shell health checkers
Diffs (updated)
-----
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122678
-----------------------------------------------------------
Master (bab21ed) is red with this patch.
./build-support/jenkins/build.sh
virtualenv-14.0.6/virtualenv_embedded/activate.ps1
virtualenv-14.0.6/virtualenv_embedded/activate.sh
virtualenv-14.0.6/virtualenv_embedded/activate_this.py
virtualenv-14.0.6/virtualenv_embedded/deactivate.bat
virtualenv-14.0.6/virtualenv_embedded/distutils-init.py
virtualenv-14.0.6/virtualenv_embedded/distutils.cfg
virtualenv-14.0.6/virtualenv_embedded/python-config
virtualenv-14.0.6/virtualenv_embedded/site.py
virtualenv-14.0.6/virtualenv_support/
virtualenv-14.0.6/virtualenv_support/__init__.py
virtualenv-14.0.6/virtualenv_support/argparse-1.4.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/setuptools-20.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/wheel-0.29.0-py2.py3-none-any.whl
+ touch virtualenv-14.0.6/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.6/virtualenv.py /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv
New python executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python2.7
Also creating executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python
Installing setuptools, pip, wheel...done.
Collecting isort==4.0.0
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:315: SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name Indication) extension to TLS is not available on this platform. This may cause the server to present an incorrect TLS certificate, which can cause validation failures. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#snimissingwarning.
SNIMissingWarning
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:120: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
InsecurePlatformWarning
Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py Imports are incorrectly sorted.
--- /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py:before 2016-03-09 05:21:26.749548
+++ /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py:after 2016-03-09 05:32:48.681704
@@ -19,7 +19,6 @@
from mesos.interface.mesos_pb2 import TaskState
from pystachio import Environment, String
-
from twitter.common import log
from twitter.common.exceptions import ExceptionalThread
from twitter.common.metrics import LambdaGauge
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 9, 2016, 5:11 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 5:11 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 9, 2016, 5:11 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing DSL defined variables to shell health checkers
Diffs (updated)
-----
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122671
-----------------------------------------------------------
Master (bab21ed) is red with this patch.
./build-support/jenkins/build.sh
virtualenv-14.0.6/virtualenv_embedded/distutils-init.py
virtualenv-14.0.6/virtualenv_embedded/distutils.cfg
virtualenv-14.0.6/virtualenv_embedded/python-config
virtualenv-14.0.6/virtualenv_embedded/site.py
virtualenv-14.0.6/virtualenv_support/
virtualenv-14.0.6/virtualenv_support/__init__.py
virtualenv-14.0.6/virtualenv_support/argparse-1.4.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/pip-8.0.2-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/setuptools-20.0-py2.py3-none-any.whl
virtualenv-14.0.6/virtualenv_support/wheel-0.29.0-py2.py3-none-any.whl
+ touch virtualenv-14.0.6/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-14.0.6/virtualenv.py /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv
New python executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python2.7
Also creating executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python
Installing setuptools, pip, wheel...done.
Collecting isort==4.0.0
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:315: SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name Indication) extension to TLS is not available on this platform. This may cause the server to present an incorrect TLS certificate, which can cause validation failures. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#snimissingwarning.
SNIMissingWarning
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:120: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
InsecurePlatformWarning
Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py Imports are incorrectly sorted.
--- /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py:before 2016-03-09 04:31:26.677371
+++ /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/aurora/executor/common/health_checker.py:after 2016-03-09 04:41:11.277559
@@ -18,10 +18,7 @@
import traceback
from mesos.interface.mesos_pb2 import TaskState
-from pystachio import (
- Environment,
- String,
-)
+from pystachio import Environment, String
from twitter.common import log
from twitter.common.exceptions import ExceptionalThread
from twitter.common.metrics import LambdaGauge
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 9, 2016, 4:24 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 4:24 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing DSL defined variables to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing DSL defined variables to shell
health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 9, 2016, 4:24 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Summary (updated)
-----------------
Exposing DSL defined variables to shell health checkers
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description (updated)
-------
Exposing DSL defined variables to shell health checkers
Diffs (updated)
-----
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122581
-----------------------------------------------------------
Master (26efe55) is red with this patch.
./build-support/jenkins/build.sh
[1m proxy_driver = ProxyDriver()[0m
[1m with temporary_dir() as checkpoint_root:[0m
[1m te = AuroraExecutor([0m
[1m> runner_provider=make_provider(checkpoint_root),[0m
[1m sandbox_provider=DefaultTestSandboxProvider())[0m
src/test/python/apache/aurora/executor/test_thermos_executor.py:580:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in make_provider
[1m pex_location=thermos_runner_path(),[0m
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
build = True
[1m def thermos_runner_path(build=True):[0m
[1m if not build:[0m
[1m return getattr(thermos_runner_path, 'value', None)[0m
[1m [0m
[1m if not hasattr(thermos_runner_path, 'value'):[0m
[1m pex_dir = safe_mkdtemp()[0m
[1m> assert subprocess.call(["./pants", "--pants-distdir=%s" % pex_dir, "binary",[0m
[1m "src/main/python/apache/thermos/runner:thermos_runner"]) == 0[0m
[1m[31mE assert 1 == 0[0m
[1m[31mE + where 1 = <function call at 0x7f9dea0b8938>(['./pants', '--pants-distdir=/tmp/user/20000/tmpZBVjJy', 'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])[0m
[1m[31mE + where <function call at 0x7f9dea0b8938> = subprocess.call[0m
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: AssertionError
-------------- Captured stderr call --------------
Traceback (most recent call last):
File "/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.71/bin/pants", line 7, in <module>
from pants.bin.pants_exe import main
ImportError: No module named pants.bin.pants_exe
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 16 failed, 637 passed, 5 skipped, 1 warnings, 8 error in 273.68 seconds [0m
FAILURE
19:23:56 06:00 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 8, 2016, 6:32 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 6:32 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 8, 2016, 6:32 p.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing ports to shell health checkers
Diffs (updated)
-----
NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122469
-----------------------------------------------------------
Ship it!
Master (a91a759) 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 8, 2016, 2:10 a.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 2:10 a.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
> docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 8, 2016, 2:10 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing ports to shell health checkers
Diffs (updated)
-----
NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/
-----------------------------------------------------------
(Updated March 8, 2016, 2:08 a.m.)
Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
Bugs: AURORA-1622
https://issues.apache.org/jira/browse/AURORA-1622
Repository: aurora
Description
-------
Exposing ports to shell health checkers
Diffs (updated)
-----
NEWS b84a94550f93691eba0220afedb2bb4d5e00e6bd
docs/configuration-reference.md 10702ff4e700b6da7bdd7fd036de442be1eba45c
src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
Diff: https://reviews.apache.org/r/44486/diff/
Testing
-------
Unit and end to end test.
Thanks,
Dmitriy Shirchenko
Re: Review Request 44486: Exposing ports to shell health checkers
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44486/#review122462
-----------------------------------------------------------
This code lgtm, but adding some docs that define what a shell health checker can expect in its environment from thermos would be good. Maybe [here](https://github.com/apache/aurora/blob/master/docs/configuration-reference.md#shellhealthchecker-objects). This also seems [NEWS](https://github.com/apache/aurora/blob/master/NEWS) worthy.
- John Sirois
On March 7, 2016, 6:42 p.m., Dmitriy Shirchenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44486/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 6:42 p.m.)
>
>
> Review request for Aurora, John Sirois, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-1622
> https://issues.apache.org/jira/browse/AURORA-1622
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Exposing ports to shell health checkers
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/common/health_check/shell.py 890bf0c5d50d0022c044a37191a2e3145cc6340f
> src/main/python/apache/aurora/executor/common/health_checker.py 303972778baa04e9d7dd47fb208fe1427e779976
> src/test/python/apache/aurora/common/health_check/test_shell.py 84f717fbf724c11863b4980fd2740dc23fe1404e
> src/test/python/apache/aurora/executor/common/test_health_checker.py 9bebce8f5a26662f58075d7ce881a8bdacb2fe46
>
> Diff: https://reviews.apache.org/r/44486/diff/
>
>
> Testing
> -------
>
> Unit and end to end test.
>
>
> Thanks,
>
> Dmitriy Shirchenko
>
>