Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-21 Thread Akash Gupta 

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

(Updated Feb. 22, 2018, 1:45 a.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and Gaston 
Kleiman.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Now with the health check library refactor, the wrapping of `docker
exec` for docker command health checks can be easily moved inside
the library.


Diffs (updated)
-

  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
  src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 


Diff: https://reviews.apache.org/r/65396/diff/6/

Changes: https://reviews.apache.org/r/65396/diff/5-6/


Testing
---

make check


Thanks,

Akash Gupta

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-21 Thread Alexander Rukletsov 

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


Ship it!




Ship It!

- Alexander Rukletsov


On Feb. 8, 2018, 5:50 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Feb. 8, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-20 Thread Joseph Wu 

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 8, 2018, 9:50 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Feb. 8, 2018, 9:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-12 Thread Andrew Schwartzmeyer 

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Feb. 8, 2018, 9:50 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Feb. 8, 2018, 9:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-08 Thread Akash Gupta 

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

(Updated Feb. 8, 2018, 5:50 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and Gaston 
Kleiman.


Changes
---

rebased


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


Repository: mesos


Description
---

Now with the health check library refactor, the wrapping of `docker
exec` for docker command health checks can be easily moved inside
the library.


Diffs (updated)
-

  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
  src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 


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

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


Testing
---

make check


Thanks,

Akash Gupta

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-08 Thread Akash Gupta 


> On Feb. 2, 2018, 9:38 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.cpp
> > Lines 523-525 (patched)
> > 
> >
> > It's kind of funny they're quoting this. Not something to fix here, but 
> > it's weird to have hard-coded `sh -c`.
> > 
> > Joe and I have talked about making the shell switchable (so you can 
> > switch between cmd.exe and PowerShell on Windows, or sh and bash etc. on 
> > Linux). Maybe we should file an issue so can start tracking hard-coded 
> > shells with a `TODO`?

Yeah that sounds good to me. It's going to call os::Shell in the next patch, so 
the hard-coded `sh -c` is going to go away.


> On Feb. 2, 2018, 9:38 p.m., Andrew Schwartzmeyer wrote:
> > src/checks/checker_process.cpp
> > Lines 529 (patched)
> > 
> >
> > Is `command.arguments()` iterable? If so, we could use the vector's 
> > reange constructor.

we can do `commandArguments.insert(commandArguments.end(), 
command.arguments().begin(), command.arguments().end());`. But since it's going 
to changed anyway in the next patch, I'll do it there.


- Akash


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


On Feb. 8, 2018, 5:50 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Feb. 8, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65396: Moved docker command check code inside health check library.

2018-02-02 Thread Andrew Schwartzmeyer 

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




src/checks/checker_process.cpp
Lines 523-525 (patched)


It's kind of funny they're quoting this. Not something to fix here, but 
it's weird to have hard-coded `sh -c`.

Joe and I have talked about making the shell switchable (so you can switch 
between cmd.exe and PowerShell on Windows, or sh and bash etc. on Linux). Maybe 
we should file an issue so can start tracking hard-coded shells with a `TODO`?



src/checks/checker_process.cpp
Lines 529 (patched)


Is `command.arguments()` iterable? If so, we could use the vector's reange 
constructor.


- Andrew Schwartzmeyer


On Jan. 29, 2018, 10:21 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65396/
> ---
> 
> (Updated Jan. 29, 2018, 10:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now with the health check library refactor, the wrapping of `docker
> exec` for docker command health checks can be easily moved inside
> the library.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65396/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>