You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/04/04 22:25:37 UTC

Review Request 58196: Implemented TCP check support in command and default executors.

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

Review request for mesos, Gast�n Kleiman and Vinod Kone.


Bugs: MESOS-7275
    https://issues.apache.org/jira/browse/MESOS-7275


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
  src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 


Diff: https://reviews.apache.org/r/58196/diff/1/


Testing
-------

make check on Mac OS 10.11.6 and various linux variants.


Thanks,

Alexander Rukletsov


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On April 7, 2017, 2:39 p.m., Gast�n Kleiman wrote:
> > src/checks/checker.cpp
> > Lines 1163-1164 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1163>
> >
> >     I'd add comments saying that this is the stderr/stdout of the TCP checker process.
> >     
> >     We should also change the logging level to `VLOG(1)` and also do something similar for `HTTP` checks that use `curl`.

Good catch, Gast�n!


- Alexander


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


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/#review171346
-----------------------------------------------------------




src/checks/checker.cpp
Lines 1163-1164 (patched)
<https://reviews.apache.org/r/58196/#comment244250>

    I'd add comments saying that this is the stderr/stdout of the TCP checker process.
    
    We should also change the logging level to `VLOG(1)` and also do something similar for `HTTP` checks that use `curl`.


- Gast�n Kleiman


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1166-1169 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1166>
> >
> >     It's a bit unfortunate that the `errno` returned by `connect` is subsumed by the tcp check command. But I guess there is no easy way to expose that to the scheduler.
> 
> Alexander Rukletsov wrote:
>     I think that the scheduler is not really interested in `errno`. What information can be interesting for a scheduler, is whether
>     - a check command was configured properly and succeeded to run, but the check result was `false`,
>     - or there was a problem with the check command itself.
>     
>     We can distinguish these cases in our API by returning `CheckStatusInfo.Tcp` with absent `succeeded` field. The question is, which errors shall belong to which category. For example, connection timeouts, or socket creation failures. I'm not too worried about this, because we can relatively easy adjust later, since the API is already in place.

My suggestion regarding `errno` was basically trying to figure out if we can give more fine grained information about how a tcp check performed not merely that it succeeded/failed; similar to how we provide explicit status codes for HTTP check. But, yea we can add that later.


- Vinod


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


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1081 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1081>
> >
> >     Does this work even when executor is running with its own file system?

This is a good question. My understanding is that an executor always have access to `launcherDir` because it's necessary for `mesos-containerizer` command, which we, e.g., put into `pre_exec_commands`. However, this might not be the case for the default executor, because launch is delegated to the agent in this case. @Jie, @Gilbert, any comments?


- Alexander


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


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1166-1169 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1166>
> >
> >     It's a bit unfortunate that the `errno` returned by `connect` is subsumed by the tcp check command. But I guess there is no easy way to expose that to the scheduler.

I think that the scheduler is not really interested in `errno`. What information can be interesting for a scheduler, is whether
- a check command was configured properly and succeeded to run, but the check result was `false`,
- or there was a problem with the check command itself.

We can distinguish these cases in our API by returning `CheckStatusInfo.Tcp` with absent `succeeded` field. The question is, which errors shall belong to which category. For example, connection timeouts, or socket creation failures. I'm not too worried about this, because we can relatively easy adjust later, since the API is already in place.


- Alexander


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


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Jie Yu <yu...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1081 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1081>
> >
> >     Does this work even when executor is running with its own file system?
> 
> Alexander Rukletsov wrote:
>     This is a good question. My understanding is that an executor always have access to `launcherDir` because it's necessary for `mesos-containerizer` command, which we, e.g., put into `pre_exec_commands`. However, this might not be the case for the default executor, because launch is delegated to the agent in this case. @Jie, @Gilbert, any comments?

Both default executor and command executor can see agent's filesystem. Is the expectation here that the checker will only be used by these two executors?


- Jie


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


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1081 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1081>
> >
> >     Does this work even when executor is running with its own file system?
> 
> Alexander Rukletsov wrote:
>     This is a good question. My understanding is that an executor always have access to `launcherDir` because it's necessary for `mesos-containerizer` command, which we, e.g., put into `pre_exec_commands`. However, this might not be the case for the default executor, because launch is delegated to the agent in this case. @Jie, @Gilbert, any comments?
> 
> Jie Yu wrote:
>     Both default executor and command executor can see agent's filesystem. Is the expectation here that the checker will only be used by these two executors?
> 
> Vinod Kone wrote:
>     Yea checker is only used by default and command executors. So I think we are ok here.

We might add support for docker executor in the future. I'll add a note here.


- Alexander


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


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Vinod Kone <vi...@gmail.com>.

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1081 (patched)
> > <https://reviews.apache.org/r/58196/diff/1/?file=1684610#file1684610line1081>
> >
> >     Does this work even when executor is running with its own file system?
> 
> Alexander Rukletsov wrote:
>     This is a good question. My understanding is that an executor always have access to `launcherDir` because it's necessary for `mesos-containerizer` command, which we, e.g., put into `pre_exec_commands`. However, this might not be the case for the default executor, because launch is delegated to the agent in this case. @Jie, @Gilbert, any comments?
> 
> Jie Yu wrote:
>     Both default executor and command executor can see agent's filesystem. Is the expectation here that the checker will only be used by these two executors?

Yea checker is only used by default and command executors. So I think we are ok here.


- Vinod


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


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/#review171307
-----------------------------------------------------------




src/checks/checker.hpp
Lines 64 (patched)
<https://reviews.apache.org/r/58196/#comment244189>

    std::string&



src/checks/checker.hpp
Lines 91 (patched)
<https://reviews.apache.org/r/58196/#comment244190>

    std::string&



src/checks/checker.cpp
Lines 138 (patched)
<https://reviews.apache.org/r/58196/#comment244191>

    string&



src/checks/checker.cpp
Lines 210 (patched)
<https://reviews.apache.org/r/58196/#comment244192>

    s/a binary/the binary/



src/checks/checker.cpp
Lines 234 (patched)
<https://reviews.apache.org/r/58196/#comment244193>

    string&



src/checks/checker.cpp
Lines 263 (patched)
<https://reviews.apache.org/r/58196/#comment244194>

    string&



src/checks/checker.cpp
Lines 318 (patched)
<https://reviews.apache.org/r/58196/#comment244195>

    string&



src/checks/checker.cpp
Lines 1081 (patched)
<https://reviews.apache.org/r/58196/#comment244196>

    Does this work even when executor is running with its own file system?



src/checks/checker.cpp
Lines 1088 (patched)
<https://reviews.apache.org/r/58196/#comment244197>

    s/tcpCommand/command/



src/checks/checker.cpp
Lines 1090 (patched)
<https://reviews.apache.org/r/58196/#comment244198>

    s/tcpCommandArguments/argv/



src/checks/checker.cpp
Lines 1166-1169 (patched)
<https://reviews.apache.org/r/58196/#comment244200>

    It's a bit unfortunate that the `errno` returned by `connect` is subsumed by the tcp check command. But I guess there is no easy way to expose that to the scheduler.



src/tests/check_tests.cpp
Lines 947 (patched)
<https://reviews.apache.org/r/58196/#comment244201>

    Same comment as previous review. This can result in flaky test.



src/tests/check_tests.cpp
Lines 959 (patched)
<https://reviews.apache.org/r/58196/#comment244202>

    `has_succeeded` sounds weird here :) more reason to change the field name.



src/tests/check_tests.cpp
Lines 1913 (patched)
<https://reviews.apache.org/r/58196/#comment244203>

    ditto.


- Vinod Kone


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/#review171079
-----------------------------------------------------------



Patch looks great!

Reviews applied: [58190, 58191, 58192, 58193, 58194, 58195, 58196]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/
-----------------------------------------------------------

(Updated April 24, 2017, 10:12 a.m.)


Review request for mesos, Gast�n Kleiman and Vinod Kone.


Bugs: MESOS-7275
    https://issues.apache.org/jira/browse/MESOS-7275


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/checks/checker.hpp fec30a22494c2684377c579b601cac1f4e909608 
  src/checks/checker.cpp cf7a086ead4413083ea1e28112f1f22dc18f0a89 
  src/launcher/default_executor.cpp d003c1b307c0c258fd82028ea7d932d92653e746 
  src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
  src/tests/check_tests.cpp 72fa64c6149dbb12ab5e2513b35bee31d122a39a 


Diff: https://reviews.apache.org/r/58196/diff/4/

Changes: https://reviews.apache.org/r/58196/diff/3-4/


Testing
-------

make check on Mac OS 10.11.6 and various linux variants.


Thanks,

Alexander Rukletsov


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/#review172473
-----------------------------------------------------------


Fix it, then Ship it!





src/checks/checker.cpp
Lines 1104 (patched)
<https://reviews.apache.org/r/58196/#comment245590>

    I think you're missing a space after "port".


- Gast�n Kleiman


On April 19, 2017, 3:16 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 19, 2017, 3:16 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fec30a22494c2684377c579b601cac1f4e909608 
>   src/checks/checker.cpp cf7a086ead4413083ea1e28112f1f22dc18f0a89 
>   src/launcher/default_executor.cpp d003c1b307c0c258fd82028ea7d932d92653e746 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp 79ba5eb38b6e7338392fb17ad39f6cd250f87d88 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/3/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/
-----------------------------------------------------------

(Updated April 19, 2017, 3:16 p.m.)


Review request for mesos, Gast�n Kleiman and Vinod Kone.


Bugs: MESOS-7275
    https://issues.apache.org/jira/browse/MESOS-7275


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/checks/checker.hpp fec30a22494c2684377c579b601cac1f4e909608 
  src/checks/checker.cpp cf7a086ead4413083ea1e28112f1f22dc18f0a89 
  src/launcher/default_executor.cpp d003c1b307c0c258fd82028ea7d932d92653e746 
  src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
  src/tests/check_tests.cpp 79ba5eb38b6e7338392fb17ad39f6cd250f87d88 


Diff: https://reviews.apache.org/r/58196/diff/3/

Changes: https://reviews.apache.org/r/58196/diff/2-3/


Testing
-------

make check on Mac OS 10.11.6 and various linux variants.


Thanks,

Alexander Rukletsov


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/#review171360
-----------------------------------------------------------



Patch looks great!

Reviews applied: [58190, 58191, 58192, 58193, 58194, 58195, 58196]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
>     https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> -------
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58196: Implemented TCP check support in command and default executors.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58196/
-----------------------------------------------------------

(Updated April 7, 2017, 3:25 p.m.)


Review request for mesos, Gast�n Kleiman and Vinod Kone.


Changes
-------

Addressed some comments.


Bugs: MESOS-7275
    https://issues.apache.org/jira/browse/MESOS-7275


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
  src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 


Diff: https://reviews.apache.org/r/58196/diff/2/

Changes: https://reviews.apache.org/r/58196/diff/1-2/


Testing
-------

make check on Mac OS 10.11.6 and various linux variants.


Thanks,

Alexander Rukletsov