Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

> On Oct. 30, 2015, 2:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 190
> > 
> >
> > OK, this is problematic and almost caused us an incident at Twitter.
> > 
> > Mesos is able to create 'freezer' hierarchy if it's not already 
> > mounted. In some configurations, the host won't pre-mount all cgroups 
> > hierarchies. We should remove that check here.
> 
> Jie Yu wrote:
> Chatted with Artem, a better way is to check: cgroups::enabled("freezer") 
> here

https://reviews.apache.org/r/39841/


- Artem


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


On Oct. 23, 2015, 4:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 4:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Jie Yu]

> On Oct. 30, 2015, 9:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 190
> > 
> >
> > OK, this is problematic and almost caused us an incident at Twitter.
> > 
> > Mesos is able to create 'freezer' hierarchy if it's not already 
> > mounted. In some configurations, the host won't pre-mount all cgroups 
> > hierarchies. We should remove that check here.

Chatted with Artem, a better way is to check: cgroups::enabled("freezer") here


- Jie


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


On Oct. 23, 2015, 11:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 11:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Jie Yu]

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



src/slave/containerizer/linux_launcher.cpp (line 190)


OK, this is problematic and almost caused us an incident at Twitter.

Mesos is able to create 'freezer' hierarchy if it's not already mounted. In 
some configurations, the host won't pre-mount all cgroups hierarchies. We 
should remove that check here.


- Jie Yu


On Oct. 23, 2015, 11:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 11:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

> On Oct. 26, 2015, 11:55 a.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.hpp, line 39
> > 
> >
> > Cna you rename it to 'available'? We tend to omit 'is' or 'are' for 
> > those functions (e.g., cgroups::enabled(), not cgroups::areEnabled()).
> 
> Jie Yu wrote:
> I fixed it for you. The patch is committed.

Thanks, Jie!


- Artem


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


On Oct. 23, 2015, 4:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 4:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Jie Yu]

> On Oct. 26, 2015, 6:55 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.hpp, line 39
> > 
> >
> > Cna you rename it to 'available'? We tend to omit 'is' or 'are' for 
> > those functions (e.g., cgroups::enabled(), not cgroups::areEnabled()).

I fixed it for you. The patch is committed.


- Jie


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


On Oct. 23, 2015, 11:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 11:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Jie Yu]

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

Ship it!


Thanks!


src/slave/containerizer/linux_launcher.hpp (line 39)


Cna you rename it to 'available'? We tend to omit 'is' or 'are' for those 
functions (e.g., cgroups::enabled(), not cgroups::areEnabled()).


- Jie Yu


On Oct. 23, 2015, 11:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 11:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

> On Oct. 23, 2015, 11:42 a.m., Neil Conway wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 189
> > 
> >
> > You could simplify this to "return freezer.isSome();". Clearly more 
> > concise; debatable whether it is more reasonable, although I'd say it would 
> > be an improvement.

Indeed! Thank you!


- Artem


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


On Oct. 23, 2015, 11:50 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 11:50 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joris Van Remoortere, and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

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

(Updated Oct. 23, 2015, 11:50 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Kapil Arya.


Repository: mesos


Description
---

We recently switched to using LinuxLauncher by default 
(https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
 This causes problems with running the slave in a Docker container. 

This patch introduces a function for checking whether the machine is suited for 
running Linux launcher. For the time being it checks whether the freezer cgroup 
subsystem enabled, as the Linux launcher evolves we might want to add more 
checks.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
35b3315498d690ed66616617aa7d51455371fb5b 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 

Diff: https://reviews.apache.org/r/39604/diff/


Testing
---

- Ran Mesoss tests in a Docker container where cgroup was not available.
- Ran the new Jenkins script (https://reviews.apache.org/r/37787/).


Thanks,

Artem Harutyunyan

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

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

(Updated Oct. 23, 2015, 3:21 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
Arya.


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


Repository: mesos


Description
---

We recently switched to using LinuxLauncher by default 
(https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
 This causes problems with running the slave in a Docker container. 

This patch introduces a function for checking whether the machine is suited for 
running Linux launcher. For the time being it checks whether the freezer cgroup 
subsystem enabled, as the Linux launcher evolves we might want to add more 
checks.


Diffs
-

  src/slave/containerizer/linux_launcher.hpp 
35b3315498d690ed66616617aa7d51455371fb5b 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 

Diff: https://reviews.apache.org/r/39604/diff/


Testing
---

- Ran Mesoss tests in a Docker container where cgroup was not available.
- Ran the new Jenkins script (https://reviews.apache.org/r/37787/).


Thanks,

Artem Harutyunyan

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Till Toenshoff]

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



src/slave/containerizer/linux_launcher.cpp (lines 187 - 190)


It is pretty well readable already, hence this point is VERY minor...

I believe this should be formatted like any other boolean expression, not a 
function definition or invocation.
Please see 
http://google.github.io/styleguide/cppguide.html#Boolean_Expressions which we 
inherit here.


- Till Toenshoff


On Oct. 23, 2015, 10:21 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 10:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Till Toenshoff]

> On Oct. 23, 2015, 11:18 p.m., Till Toenshoff wrote:
> > src/slave/containerizer/linux_launcher.cpp, lines 187-190
> > 
> >
> > It is pretty well readable already, hence this point is VERY minor...
> > 
> > I believe this should be formatted like any other boolean expression, 
> > not a function definition or invocation.
> > Please see 
> > http://google.github.io/styleguide/cppguide.html#Boolean_Expressions which 
> > we inherit here.

```
  return ::geteuid() == 0 && cgroups::enabled() &&
 cgroups::hierarchy("freezer").isSome();
```

or 

```
  return ::geteuid() == 0 &&
 cgroups::enabled() &&
 cgroups::hierarchy("freezer").isSome();
```


- Till


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


On Oct. 23, 2015, 10:21 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 10:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Adam B]

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



src/slave/containerizer/linux_launcher.cpp (line 181)


Nit: Wrap '{' to next line, like all function implementations


- Adam B


On Oct. 23, 2015, 3:21 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 3:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

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

(Updated Oct. 23, 2015, 4:48 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
Arya.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

We recently switched to using LinuxLauncher by default 
(https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
 This causes problems with running the slave in a Docker container. 

This patch introduces a function for checking whether the machine is suited for 
running Linux launcher. For the time being it checks whether the freezer cgroup 
subsystem enabled, as the Linux launcher evolves we might want to add more 
checks.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
35b3315498d690ed66616617aa7d51455371fb5b 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 

Diff: https://reviews.apache.org/r/39604/diff/


Testing
---

- Ran Mesoss tests in a Docker container where cgroup was not available.
- Ran the new Jenkins script (https://reviews.apache.org/r/37787/).


Thanks,

Artem Harutyunyan

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

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

(Updated Oct. 23, 2015, 2:58 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
Arya.


Repository: mesos


Description
---

We recently switched to using LinuxLauncher by default 
(https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
 This causes problems with running the slave in a Docker container. 

This patch introduces a function for checking whether the machine is suited for 
running Linux launcher. For the time being it checks whether the freezer cgroup 
subsystem enabled, as the Linux launcher evolves we might want to add more 
checks.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
35b3315498d690ed66616617aa7d51455371fb5b 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 

Diff: https://reviews.apache.org/r/39604/diff/


Testing
---

- Ran Mesoss tests in a Docker container where cgroup was not available.
- Ran the new Jenkins script (https://reviews.apache.org/r/37787/).


Thanks,

Artem Harutyunyan

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Kapil Arya]

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

Ship it!


Ship It!

- Kapil Arya


On Oct. 23, 2015, 5:58 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

[Artem Harutyunyan]

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

(Updated Oct. 23, 2015, 2:35 p.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Kapil Arya.


Changes
---

Addressed comments.


Repository: mesos


Description
---

We recently switched to using LinuxLauncher by default 
(https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
 This causes problems with running the slave in a Docker container. 

This patch introduces a function for checking whether the machine is suited for 
running Linux launcher. For the time being it checks whether the freezer cgroup 
subsystem enabled, as the Linux launcher evolves we might want to add more 
checks.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
35b3315498d690ed66616617aa7d51455371fb5b 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 

Diff: https://reviews.apache.org/r/39604/diff/


Testing
---

- Ran Mesoss tests in a Docker container where cgroup was not available.
- Ran the new Jenkins script (https://reviews.apache.org/r/37787/).


Thanks,

Artem Harutyunyan