Review Request 32891: Support for entering and configuring a Linux chroot.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32891/ --- Review request for mesos, Chi Zhang, Jay Buffington, Jie Yu, and James Peach. Bugs: MESOS-2350 https://issues.apache.org/jira/browse/MESOS-2350 Repository: mesos Description --- Support for entering and configuring a Linux chroot. Diffs - src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 Diff: https://reviews.apache.org/r/32891/diff/ Testing --- Thanks, Ian Downes
Re: Review Request 32859: Add Camel-case libprocess variable and method names sample.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32859/#review78934 --- Patch looks great! Reviews applied: [32859] All tests passed. - Mesos ReviewBot On April 6, 2015, 4:36 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32859/ --- (Updated April 6, 2015, 4:36 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-2576 https://issues.apache.org/jira/browse/MESOS-2576 Repository: mesos Description --- Add Camel-case libprocess variable and method names sample. Diffs - 3rdparty/libprocess/src/libev.hpp e4a403d9e769c13182f26034e0dd1c4db92b04cb 3rdparty/libprocess/src/libev.cpp 610dfb896ed8f9c00f9cf8fc8dbfc7d434f7d1e5 3rdparty/libprocess/src/libev_poll.cpp 324e4dd950989f3717ca73fe48520ca3e518518f 3rdparty/libprocess/src/process.cpp cf4e36489be2c6aa01e838c1c71383f248deab5b Diff: https://reviews.apache.org/r/32859/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/
---
(Updated April 6, 2015, 11:02 a.m.)
Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, and
James Peach.
Changes
---
Moved code out of the launch helper and into linux/fs.
Support chrooting on non Linux platforms using just POSIX chroot.
Removed some of the implementation specific checks (they'll be done by the
caller).
Manual testing of just the mesos-containerizer helper binary can be done with:
{noformat}
[idownes:hostname build]$ 3/dev/zero 4/dev/null
./src/.libs/mesos-containerizer launch --pipe_read=3 --pipe_write=4
--directory=XXX --command=file:///path/to/commandinfo.json
--rootfs=/path/to/a/chroot
{noformat}
Manually tested with basic Linux and OSX chroots. Proper tests will follow.
Bugs: MESOS-2350
https://issues.apache.org/jira/browse/MESOS-2350
Repository: mesos
Description
---
Optionally take a path that the launch helper should chroot to before exec'ing
the executor. It is assumed that the work directory is mounted to the
appropriate location under the chroot. In particular, the path to the executor
must be relative to the chroot.
Configuration that should be private to the chroot is done during the launch,
e.g. mounting proc and statically configuring basic devices. It is assumed that
other configuration, e.g., preparing the image, mounting in volumes or
persistent resources, is done by the caller.
Mounts can be made to the chroot (e.g., updating the volumes or persistent
resources) and they will propagate in to the container but mounts made inside
the container will not propagate out to the host.
It currently assumes that at least {{chroot}}/tmp is writeable and that mount
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
This is specific to Linux.
Diffs (updated)
-
src/slave/containerizer/mesos/launch.hpp
7c8b535746b5ce9add00afef86fdb6faefb5620e
src/slave/containerizer/mesos/launch.cpp
2f2d60e2011f60ec711d3b29fd2c157e30c83c34
Diff: https://reviews.apache.org/r/31444/diff/
Testing
---
Manual testing only so far. This is harder to automate because we need a
self-contained chroot to execute something in... Suggestions welcome.
Thanks,
Ian Downes
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 7:27 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 support/timed_tests.sh 5e01af9411d1d736b12a6996c3ecb0f18468faca Diff: https://reviews.apache.org/r/32898/diff/ Testing (updated) --- make check (on linux host only) Thanks, Paul Brett
Re: Review Request 32895: Environment variables are case sensitive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32895/#review78974 --- Patch looks great! Reviews applied: [32895] All tests passed. - Mesos ReviewBot On April 6, 2015, 6:45 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32895/ --- (Updated April 6, 2015, 6:45 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1801 https://issues.apache.org/jira/browse/mesos-1801 Repository: mesos Description --- Environment variables are case sensitive Diffs - src/deploy/mesos-master-env.sh.template 7fe7e536de351c9c8673d9bb8b16b59926e00c2c src/deploy/mesos-slave-env.sh.template 31567d6a47e19385aed56edfc7e67457c8cdde3e Diff: https://reviews.apache.org/r/32895/diff/ Testing --- Thanks, Paul Brett
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/#review79053 --- Patch looks great! Reviews applied: [32898] All tests passed. - Mesos ReviewBot On April 6, 2015, 7:27 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 7:27 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 support/timed_tests.sh 5e01af9411d1d736b12a6996c3ecb0f18468faca Diff: https://reviews.apache.org/r/32898/diff/ Testing --- make check (on linux host only) Thanks, Paul Brett
Ticket cleanup
bcc dev@ Hi, As part of FixIt week at Twitter we are doing ticket cleanup of Mesos. So you might see a deluge of emails on issues@. Apologies for any convenience, Vinod
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/#review79055 --- Can you please split this into two separate reviews, one for the ec2 scripts and one for the rest of the codebase. ec2/deploy.amazon64-old/root/mesos-ec2/setup https://reviews.apache.org/r/32898/#comment128095 This is a strange usage of echo or printf. If we insist on this technique to approve keys, what about using /usr/bin/true? src/tests/port_mapping_tests.cpp https://reviews.apache.org/r/32898/#comment128098 The argument doesn't need to be quoted if there's no formatting required? Regardless, should stay consistent with double quotes used elsewhere? - Ian Downes On April 6, 2015, 12:27 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 12:27 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 support/timed_tests.sh 5e01af9411d1d736b12a6996c3ecb0f18468faca Diff: https://reviews.apache.org/r/32898/diff/ Testing --- make check (on linux host only) Thanks, Paul Brett
Regarding old frameworks in Mesos repository
There exist a couple of frameworks in the Mesos codebase under /frameworks: deploy_jar haproxy+apache mesos-submit torque (See https://github.com/apache/mesos/tree/master/frameworks) Anyone still uses them? These frameworks are not trivial implementations like the ones under src/examples to demonstrate/test Mesos features and they rely on external programs to run. Since we don't actively maintain them, they may have already stopped working with the current versions of these programs. We'd like to remove these from the Mesos repository. If there are folks who still use them and would like to contribute, the ideal place to host them is in their own repos. e.g., https://github.com/mesos/hadoop Any comments? -- Jiang Yan Xu y...@jxu.me @xujyan http://twitter.com/xujyan
Review Request 32895: Environment variables are case sensitive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32895/ --- Review request for mesos and Ian Downes. Bugs: mesos-1801 https://issues.apache.org/jira/browse/mesos-1801 Repository: mesos Description --- Environment variables are case sensitive Diffs - src/deploy/mesos-master-env.sh.template 7fe7e536de351c9c8673d9bb8b16b59926e00c2c src/deploy/mesos-slave-env.sh.template 31567d6a47e19385aed56edfc7e67457c8cdde3e Diff: https://reviews.apache.org/r/32895/diff/ Testing --- Thanks, Paul Brett
Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/#review78965
---
Patch looks great!
Reviews applied: [32891, 31444]
All tests passed.
- Mesos ReviewBot
On April 6, 2015, 6:02 p.m., Ian Downes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/
---
(Updated April 6, 2015, 6:02 p.m.)
Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu,
and James Peach.
Bugs: MESOS-2350
https://issues.apache.org/jira/browse/MESOS-2350
Repository: mesos
Description
---
Optionally take a path that the launch helper should chroot to before
exec'ing the executor. It is assumed that the work directory is mounted to
the appropriate location under the chroot. In particular, the path to the
executor must be relative to the chroot.
Configuration that should be private to the chroot is done during the launch,
e.g. mounting proc and statically configuring basic devices. It is assumed
that other configuration, e.g., preparing the image, mounting in volumes or
persistent resources, is done by the caller.
Mounts can be made to the chroot (e.g., updating the volumes or persistent
resources) and they will propagate in to the container but mounts made inside
the container will not propagate out to the host.
It currently assumes that at least {{chroot}}/tmp is writeable and that mount
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
This is specific to Linux.
Diffs
-
src/slave/containerizer/mesos/launch.hpp
7c8b535746b5ce9add00afef86fdb6faefb5620e
src/slave/containerizer/mesos/launch.cpp
2f2d60e2011f60ec711d3b29fd2c157e30c83c34
Diff: https://reviews.apache.org/r/31444/diff/
Testing
---
Manual testing only so far. This is harder to automate because we need a
self-contained chroot to execute something in... Suggestions welcome.
Thanks,
Ian Downes
Re: Regarding old frameworks in Mesos repository
+1 to moving these out to https://github.com/mesos/framework even if they are used, in which case we should open an issue tracker for each separate project and give write permissions to that repo to anyone willing to maintain it. On Mon, Apr 6, 2015 at 12:10 PM, Yan Xu y...@jxu.me wrote: There exist a couple of frameworks in the Mesos codebase under /frameworks: deploy_jar haproxy+apache mesos-submit torque (See https://github.com/apache/mesos/tree/master/frameworks) Anyone still uses them? These frameworks are not trivial implementations like the ones under src/examples to demonstrate/test Mesos features and they rely on external programs to run. Since we don't actively maintain them, they may have already stopped working with the current versions of these programs. We'd like to remove these from the Mesos repository. If there are folks who still use them and would like to contribute, the ideal place to host them is in their own repos. e.g., https://github.com/mesos/hadoop Any comments? -- Jiang Yan Xu y...@jxu.me @xujyan http://twitter.com/xujyan
Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.
On March 23, 2015, 10:10 p.m., Jay Buffington wrote:
src/slave/containerizer/mesos/launch.cpp, line 107
https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line107
This list-initialization doesn't work in gcc 4.4.7. That's the version
I had with a stock centos 6.4 box.
gcc 4.4.7 currently compiles the master branch on
github.com/apache/mesos so this is bumping up the minimum gcc required
version. I guess this c++11 feature isn't used anywhere else in the code?
I installed gcc 4.8.2 (and updated bintools) and it compiled.
Initializer lists are supported in 4.4.7, see
https://gcc.gnu.org/gcc-4.4/cxx0x_status.html, but local types for template
arguments are not, see the same link. I'll pull those out into an internal
namespace and add a TODO for when we get off supporting 4.4.7.
On March 23, 2015, 10:10 p.m., Jay Buffington wrote:
src/slave/containerizer/mesos/launch.cpp, line 67
https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line67
James brought this up in his review, it looks like you're not plumbing
through --chroot.
My read on this is that to fix MESOS-1404 the mesos-containerizer
binary was introduced options like this were passed through using
launchFlags in MesosContainerizerProcess::_launch
Is that missing here?
Nope, you're not missing anything. This patch does not include any code that
uses the introduced chroot functionality.
- Ian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/#review77524
---
On April 6, 2015, 11:02 a.m., Ian Downes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/
---
(Updated April 6, 2015, 11:02 a.m.)
Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu,
and James Peach.
Bugs: MESOS-2350
https://issues.apache.org/jira/browse/MESOS-2350
Repository: mesos
Description
---
Optionally take a path that the launch helper should chroot to before
exec'ing the executor. It is assumed that the work directory is mounted to
the appropriate location under the chroot. In particular, the path to the
executor must be relative to the chroot.
Configuration that should be private to the chroot is done during the launch,
e.g. mounting proc and statically configuring basic devices. It is assumed
that other configuration, e.g., preparing the image, mounting in volumes or
persistent resources, is done by the caller.
Mounts can be made to the chroot (e.g., updating the volumes or persistent
resources) and they will propagate in to the container but mounts made inside
the container will not propagate out to the host.
It currently assumes that at least {{chroot}}/tmp is writeable and that mount
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
This is specific to Linux.
Diffs
-
src/slave/containerizer/mesos/launch.hpp
7c8b535746b5ce9add00afef86fdb6faefb5620e
src/slave/containerizer/mesos/launch.cpp
2f2d60e2011f60ec711d3b29fd2c157e30c83c34
Diff: https://reviews.apache.org/r/31444/diff/
Testing
---
Manual testing only so far. This is harder to automate because we need a
self-contained chroot to execute something in... Suggestions welcome.
Thanks,
Ian Downes
Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.
On March 18, 2015, 12:09 p.m., Jie Yu wrote:
src/slave/containerizer/mesos/launch.cpp, lines 94-100
https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line94
The typedef is not needed to me.
Not needed in what sense: I think it really helps to describe all the mounts?
On March 18, 2015, 12:09 p.m., Jie Yu wrote:
src/slave/containerizer/mesos/launch.cpp, lines 249-251
https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line249
The comment here is a little confusing to me. Are you saying that this
is a best effort check?
Also, if we are using pid namespace but not mount namespace, which of
the 'later code' will fail?
This check was an attempt to provide a more informative error message but it's
pretty flawed: we also may be in a new mount namespace and not be pid 1 in a
new pid namespace. I'm just going to remove this check and rely on later code
to detect this.
On March 18, 2015, 12:09 p.m., Jie Yu wrote:
src/slave/containerizer/mesos/launch.cpp, lines 278-280
https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line278
Do you also want to check the case where the chroot target is not a
mount point? The current code will output:
```
Chroot target is not a shared mount
```
which is fine but not as informative as
```
Chroot target needs to be a mount point
```
added
- Ian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/#review76909
---
On April 6, 2015, 11:02 a.m., Ian Downes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31444/
---
(Updated April 6, 2015, 11:02 a.m.)
Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu,
and James Peach.
Bugs: MESOS-2350
https://issues.apache.org/jira/browse/MESOS-2350
Repository: mesos
Description
---
Optionally take a path that the launch helper should chroot to before
exec'ing the executor. It is assumed that the work directory is mounted to
the appropriate location under the chroot. In particular, the path to the
executor must be relative to the chroot.
Configuration that should be private to the chroot is done during the launch,
e.g. mounting proc and statically configuring basic devices. It is assumed
that other configuration, e.g., preparing the image, mounting in volumes or
persistent resources, is done by the caller.
Mounts can be made to the chroot (e.g., updating the volumes or persistent
resources) and they will propagate in to the container but mounts made inside
the container will not propagate out to the host.
It currently assumes that at least {{chroot}}/tmp is writeable and that mount
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
This is specific to Linux.
Diffs
-
src/slave/containerizer/mesos/launch.hpp
7c8b535746b5ce9add00afef86fdb6faefb5620e
src/slave/containerizer/mesos/launch.cpp
2f2d60e2011f60ec711d3b29fd2c157e30c83c34
Diff: https://reviews.apache.org/r/31444/diff/
Testing
---
Manual testing only so far. This is harder to automate because we need a
self-contained chroot to execute something in... Suggestions welcome.
Thanks,
Ian Downes
Re: Review Request 32001: Required a period in trailing comments in the style guide.
On March 12, 2015, 2:20 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, line 30 https://reviews.apache.org/r/32001/diff/1/?file=892420#file892420line30 Hm, isn't this captured by ending each sentence with a period above? Alexander Rukletsov wrote: Yes and no. Trailing comments are not always sentences (at least in my non-native speaker's language model), it can be for example a JIRA ticket. As an alternative we can re-phrase the statement above to something like Each comment and sentences within it must end with a period. What do you think? Michael Park wrote: Haha I didn't even catch that: End each __sentence__ with a period. Do we just mean End each comment with a period.? Alexander Rukletsov wrote: Well, techically a comment may consist of multiple sentences, which means you can write something like this: ``` // To compensate for the case where a non-strict registrar is // being used, we explicitly deny removed slaves from // re-registering // This is because a non-strict registrar cannot // enforce this // We've already told frameworks the tasks were // lost so it's important to deny the slave from re-registering. ``` But maybe we lose too much time on this minor thing. Niklas Nielsen wrote: I would support mpark's idea here: How about having an extra bullet with End each sentence with a period.? Timothy Chen wrote: To be technically correct, we need a end each sentence with some puncatuation, as you'll also commonly see this in our code: master_tests.cpp // Step 7: Ensure the slave cannot re-register! We got stuck here - BenM, would you prefer not to have the extra line? I think it is handy as we have seen the non-puncuation style bug quite a few times in comments. - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/#review76286 --- On March 26, 2015, 6:08 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/ --- (Updated March 26, 2015, 6:08 a.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- See summary. Diffs - docs/mesos-c++-style-guide.md 439fe12ad137c1bed3a21d5a097c60a48f10a4f9 Diff: https://reviews.apache.org/r/32001/diff/ Testing --- None (not a functional change). Thanks, Alexander Rukletsov
Re: Review Request 32903: Eliminate the use of 'echo -n' in EC2 scripts
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32903/#review79077 --- Patch looks great! Reviews applied: [32903] All tests passed. - Mesos ReviewBot On April 6, 2015, 10:09 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32903/ --- (Updated April 6, 2015, 10:09 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate the use of 'echo -n' in EC2 scripts Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 Diff: https://reviews.apache.org/r/32903/diff/ Testing --- None Thanks, Paul Brett
Re: Review Request 32001: Required a period in trailing comments in the style guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/#review79062 --- Ship it! docs/mesos-c++-style-guide.md https://reviews.apache.org/r/32001/#comment128116 s/, prefer period though/. Period is, however, prefered./ - Niklas Nielsen On March 26, 2015, 6:08 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/ --- (Updated March 26, 2015, 6:08 a.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- See summary. Diffs - docs/mesos-c++-style-guide.md 439fe12ad137c1bed3a21d5a097c60a48f10a4f9 Diff: https://reviews.apache.org/r/32001/diff/ Testing --- None (not a functional change). Thanks, Alexander Rukletsov
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/#review79072 --- Patch looks great! Reviews applied: [32898] All tests passed. - Mesos ReviewBot On April 6, 2015, 10:05 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 10:05 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - src/tests/port_mapping_tests.cpp 55a5e69 support/timed_tests.sh 5e01af9 Diff: https://reviews.apache.org/r/32898/diff/ Testing --- make check (on linux host only) Thanks, Paul Brett
Re: Review Request 32906: Add safety check for staged but uncommitted changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32906/#review79087 --- Patch looks great! Reviews applied: [32906] All tests passed. - Mesos ReviewBot On April 6, 2015, 11:03 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32906/ --- (Updated April 6, 2015, 11:03 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-598 https://issues.apache.org/jira/browse/mesos-598 Repository: mesos Description --- Add safety check for staged but uncommitted changes Diffs - support/post-reviews.py 2b6c479cf2bda1a93a1f9b10eb0de709aa49dcbe Diff: https://reviews.apache.org/r/32906/diff/ Testing --- vim support/post-reviews.py # edited source $ ./support/post-reviews.py Please commit or stash any changes before using post-reviews! $ git add support/post-reviews.py $ ./support/post-reviews.py Please commit staged changes before using post-reviews! $ git commit . [mesos-598 1867d05] Add safety check for staged but uncommitted changes 1 file changed, 8 insertions(+), 1 deletion(-) $ ./support/post-reviews.py Running 'rbt post' across all of ... Thanks, Paul Brett
Re: Review Request 32001: Required a period in trailing comments in the style guide.
On March 12, 2015, 9:20 p.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, line 30 https://reviews.apache.org/r/32001/diff/1/?file=892420#file892420line30 Hm, isn't this captured by ending each sentence with a period above? Alexander Rukletsov wrote: Yes and no. Trailing comments are not always sentences (at least in my non-native speaker's language model), it can be for example a JIRA ticket. As an alternative we can re-phrase the statement above to something like Each comment and sentences within it must end with a period. What do you think? Michael Park wrote: Haha I didn't even catch that: End each __sentence__ with a period. Do we just mean End each comment with a period.? Alexander Rukletsov wrote: Well, techically a comment may consist of multiple sentences, which means you can write something like this: ``` // To compensate for the case where a non-strict registrar is // being used, we explicitly deny removed slaves from // re-registering // This is because a non-strict registrar cannot // enforce this // We've already told frameworks the tasks were // lost so it's important to deny the slave from re-registering. ``` But maybe we lose too much time on this minor thing. Niklas Nielsen wrote: I would support mpark's idea here: How about having an extra bullet with End each sentence with a period.? Timothy Chen wrote: To be technically correct, we need a end each sentence with some puncatuation, as you'll also commonly see this in our code: master_tests.cpp // Step 7: Ensure the slave cannot re-register! Niklas Nielsen wrote: We got stuck here - BenM, would you prefer not to have the extra line? I think it is handy as we have seen the non-puncuation style bug quite a few times in comments. Well, I'm still a bit surprised that this level of detail is necessary. If you guys think it's helpful, the first iteration of this diff seems more succinct to me :) - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/#review76286 --- On March 26, 2015, 1:08 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32001/ --- (Updated March 26, 2015, 1:08 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- See summary. Diffs - docs/mesos-c++-style-guide.md 439fe12ad137c1bed3a21d5a097c60a48f10a4f9 Diff: https://reviews.apache.org/r/32001/diff/ Testing --- None (not a functional change). Thanks, Alexander Rukletsov
Re: Review Request 32833: Added os::signals::install to install signal handlers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32833/#review79067 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp https://reviews.apache.org/r/32833/#comment128143 Why not use TryNothing? - Ben Mahler On April 3, 2015, 9:28 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32833/ --- (Updated April 3, 2015, 9:28 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- Added os::signals::install to install signal handlers. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp 30232f50cc72a79acd21499fe7602c9bcd624ff6 Diff: https://reviews.apache.org/r/32833/diff/ Testing --- Tested in the later patch. Thanks, Jie Yu
Review Request 32903: Eliminate the use of 'echo -n' in EC2 scripts
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32903/ --- Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate the use of 'echo -n' in EC2 scripts Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 Diff: https://reviews.apache.org/r/32903/diff/ Testing --- None Thanks, Paul Brett
Re: Regarding old frameworks in Mesos repository
+1 On Mon, Apr 6, 2015 at 12:28 PM, Adam Bordelon a...@mesosphere.io wrote: +1 to moving these out to https://github.com/mesos/framework even if they are used, in which case we should open an issue tracker for each separate project and give write permissions to that repo to anyone willing to maintain it. On Mon, Apr 6, 2015 at 12:10 PM, Yan Xu y...@jxu.me wrote: There exist a couple of frameworks in the Mesos codebase under /frameworks: deploy_jar haproxy+apache mesos-submit torque (See https://github.com/apache/mesos/tree/master/frameworks) Anyone still uses them? These frameworks are not trivial implementations like the ones under src/examples to demonstrate/test Mesos features and they rely on external programs to run. Since we don't actively maintain them, they may have already stopped working with the current versions of these programs. We'd like to remove these from the Mesos repository. If there are folks who still use them and would like to contribute, the ideal place to host them is in their own repos. e.g., https://github.com/mesos/hadoop Any comments? -- Jiang Yan Xu y...@jxu.me @xujyan http://twitter.com/xujyan
Re: Regarding old frameworks in Mesos repository
+1 on removing deploy_jar, haproxy+apache, torque. For mesos-submit, it seems that this should instead be a mesos CLI command. On Mon, Apr 6, 2015 at 2:04 PM, Vinod Kone vinodk...@apache.org wrote: +1 On Mon, Apr 6, 2015 at 12:28 PM, Adam Bordelon a...@mesosphere.io wrote: +1 to moving these out to https://github.com/mesos/framework even if they are used, in which case we should open an issue tracker for each separate project and give write permissions to that repo to anyone willing to maintain it. On Mon, Apr 6, 2015 at 12:10 PM, Yan Xu y...@jxu.me wrote: There exist a couple of frameworks in the Mesos codebase under /frameworks: deploy_jar haproxy+apache mesos-submit torque (See https://github.com/apache/mesos/tree/master/frameworks) Anyone still uses them? These frameworks are not trivial implementations like the ones under src/examples to demonstrate/test Mesos features and they rely on external programs to run. Since we don't actively maintain them, they may have already stopped working with the current versions of these programs. We'd like to remove these from the Mesos repository. If there are folks who still use them and would like to contribute, the ideal place to host them is in their own repos. e.g., https://github.com/mesos/hadoop Any comments? -- Jiang Yan Xu y...@jxu.me @xujyan http://twitter.com/xujyan
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 10:05 p.m.) Review request for mesos and Ian Downes. Changes --- Eliminate use of 'echo -n' from the rests and support Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs (updated) - src/tests/port_mapping_tests.cpp 55a5e69 support/timed_tests.sh 5e01af9 Diff: https://reviews.apache.org/r/32898/diff/ Testing --- make check (on linux host only) Thanks, Paul Brett
Review Request 32906: Add safety check for staged but uncommitted changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32906/ --- Review request for mesos and Ian Downes. Bugs: mesos-598 https://issues.apache.org/jira/browse/mesos-598 Repository: mesos Description --- Add safety check for staged but uncommitted changes Diffs - support/post-reviews.py 2b6c479cf2bda1a93a1f9b10eb0de709aa49dcbe Diff: https://reviews.apache.org/r/32906/diff/ Testing --- vim support/post-reviews.py # edited source $ ./support/post-reviews.py Please commit or stash any changes before using post-reviews! $ git add support/post-reviews.py $ ./support/post-reviews.py Please commit staged changes before using post-reviews! $ git commit . [mesos-598 1867d05] Add safety check for staged but uncommitted changes 1 file changed, 8 insertions(+), 1 deletion(-) $ ./support/post-reviews.py Running 'rbt post' across all of ... Thanks, Paul Brett
Re: Review Request 29333: Add docker_sock to slave flags
On Feb. 15, 2015, 3:01 a.m., Ben Mahler wrote: src/slave/flags.hpp, line 320 https://reviews.apache.org/r/29333/diff/2/?file=824295#file824295line320 Why not docker_socket ? Why not :) - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29333/#review72520 --- On Jan. 17, 2015, 1:35 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29333/ --- (Updated Jan. 17, 2015, 1:35 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Repository: mesos Description --- Add docker_sock to slave flags Diffs - src/slave/flags.hpp a4498e6 Diff: https://reviews.apache.org/r/29333/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29334: Add option to launch docker containers with helper containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29334/ --- (Updated April 7, 2015, 12:46 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2183 https://issues.apache.org/jira/browse/MESOS-2183 Repository: mesos Description --- Add option to launch docker containers with helper containers. Diffs - src/slave/containerizer/docker.hpp b7bf54a src/slave/containerizer/docker.cpp 5f4b4ce Diff: https://reviews.apache.org/r/29334/diff/ Testing --- make, tests are fixed in next commit Thanks, Timothy Chen
Re: Review Request 29333: Add docker_socket to slave flags
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29333/ --- (Updated April 7, 2015, 12:45 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Repository: mesos Description (updated) --- Add docker_socket to slave flags Diffs - src/slave/flags.hpp 3da71af Diff: https://reviews.apache.org/r/29333/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29333: Add docker_socket to slave flags
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29333/ --- (Updated April 7, 2015, 12:45 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2183 https://issues.apache.org/jira/browse/MESOS-2183 Repository: mesos Description --- Add docker_socket to slave flags Diffs - src/slave/flags.hpp 3da71af Diff: https://reviews.apache.org/r/29333/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/#review79104 --- Patch looks great! Reviews applied: [32911] All tests passed. - Mesos ReviewBot On April 7, 2015, 12:40 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/ --- (Updated April 7, 2015, 12:40 a.m.) Review request for mesos, Benjamin Hindman and Ian Downes. Bugs: MESOS-2592 https://issues.apache.org/jira/browse/MESOS-2592 Repository: mesos Description --- During recent refactorings, executor directory ownership was delegated to the fetcher. However, the fetcher is not invoked if no URIs are present in the executor or task command. This left some of these tasks broken as the directory ownership defaulted to the mesos-slave's (root). Diffs - src/slave/containerizer/external_containerizer.cpp 1bbd61cb096771b7e4a1350079f79a20102e78f9 src/slave/paths.hpp 1618439d728ded347ec75317ce8dd998acd7ee94 src/slave/paths.cpp 01ea856aa2e628d4aee5fd31f7e49d147f740e8f src/slave/slave.cpp 521624c335b9110e12ee1ff21c3918e5af6a2bde Diff: https://reviews.apache.org/r/32911/diff/ Testing --- Functional tests with mesos-execute and make check. Have created JIRA's for introduction of more permission/user tests. Thanks, Niklas Nielsen
Re: Review Request 29328: Add option to disable docker containerizer killing orphans
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29328/ --- (Updated April 7, 2015, 12:28 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2155 https://issues.apache.org/jira/browse/MESOS-2155 Repository: mesos Description --- Add option to disable docker containerizer killing orphans Diffs - src/slave/containerizer/docker.cpp f9fb078 src/slave/flags.hpp 3da71af Diff: https://reviews.apache.org/r/29328/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29330: Integrate docker executor into containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29330/ --- (Updated April 7, 2015, 12:36 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Changes --- rebased Bugs: MESOS-2115 https://issues.apache.org/jira/browse/MESOS-2115 Repository: mesos Description --- Integrate docker executor into containerizer. Diffs (updated) - src/slave/containerizer/docker.cpp f9fb078 Diff: https://reviews.apache.org/r/29330/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29331: Re-enable docker recover test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29331/ --- (Updated April 7, 2015, 12:36 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Changes --- rebased Repository: mesos Description --- Re-enable docker recover test. Diffs (updated) - src/tests/docker_containerizer_tests.cpp c772d4c836de18b0e87636cb42200356d24ec73d Diff: https://reviews.apache.org/r/29331/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29334: Add option to launch docker containers with helper containers.
On Jan. 18, 2015, 12:18 p.m., Bernd Mathiske wrote: src/slave/containerizer/docker.hpp, line 219 https://reviews.apache.org/r/29334/diff/5/?file=824296#file824296line219 IMHO readability is subverted by prolonging the underscore scheme when there is no strict series of continuations. This code would be so much more easy to read if there were descriptive method names. Absent these, please add comments that summarize the purpose and general approach of these methods in places such as this one. The comments is in the cpp code itself, I'm not inclined to add comments in the header since this is not really used outside. On Jan. 18, 2015, 12:18 p.m., Bernd Mathiske wrote: src/slave/containerizer/docker.hpp, line 196 https://reviews.apache.org/r/29334/diff/5/?file=824296#file824296line196 At this compleity level, the comments here have begun to look like an anti-pattern that might create unwanted precedent for others to mimic. IMHO this was already the case before this patch, but the current additions exacerbate it. The naming launchInContainer helps, but it is still in line with the expected style? It seems to me that the expected style breaks down here. Ideas? I think this is going to stay until we have a better naming scheme. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29334/#review68559 --- On April 7, 2015, 12:46 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29334/ --- (Updated April 7, 2015, 12:46 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2183 https://issues.apache.org/jira/browse/MESOS-2183 Repository: mesos Description --- Add option to launch docker containers with helper containers. Diffs - src/slave/containerizer/docker.hpp b7bf54a src/slave/containerizer/docker.cpp 5f4b4ce Diff: https://reviews.apache.org/r/29334/diff/ Testing --- make, tests are fixed in next commit Thanks, Timothy Chen
Re: Review Request 32859: Add Camel-case libprocess variable and method names sample.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32859/#review79117 --- Hi haosdent, I added some higher level comments in the JIRA ticket for you :-) - Joris Van Remoortere On April 6, 2015, 9:19 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32859/ --- (Updated April 6, 2015, 9:19 p.m.) Review request for mesos, Joris Van Remoortere and Niklas Nielsen. Bugs: MESOS-2576 https://issues.apache.org/jira/browse/MESOS-2576 Repository: mesos Description --- Add Camel-case libprocess variable and method names sample. Diffs - 3rdparty/libprocess/src/libev.hpp e4a403d9e769c13182f26034e0dd1c4db92b04cb 3rdparty/libprocess/src/libev.cpp 610dfb896ed8f9c00f9cf8fc8dbfc7d434f7d1e5 3rdparty/libprocess/src/libev_poll.cpp 324e4dd950989f3717ca73fe48520ca3e518518f 3rdparty/libprocess/src/process.cpp cf4e36489be2c6aa01e838c1c71383f248deab5b Diff: https://reviews.apache.org/r/32859/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 29328: Add option to disable docker containerizer killing orphans
On March 11, 2015, 5:25 p.m., Joerg Schad wrote: src/slave/flags.hpp, line 325 https://reviews.apache.org/r/29328/diff/4/?file=824288#file824288line325 Could you add this flag also the the docs/configuration.md? This is fixed in a later commit. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29328/#review76081 --- On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29328/ --- (Updated Jan. 17, 2015, 1:26 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Repository: mesos Description --- Add option to disable docker containerizer killing orphans Diffs - src/slave/containerizer/docker.cpp 5f4b4ce src/slave/flags.hpp a4498e6 Diff: https://reviews.apache.org/r/29328/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29328: Add option to disable docker containerizer killing orphans
On Jan. 17, 2015, 1:34 a.m., Ben Mahler wrote:
Just curious, what happens to the orphans if you don't kill them? Was there
a ticket for this?
Timothy Chen wrote:
the orphans remains untouched. there is a jira ticket for adding this
flag, i can fimd it later once im next to a computer
Ben Mahler wrote:
I understood that part :)
But why is it ok to leave orphans untouched? Sounds like a bug to me.. is
there some context I'm missing here?
Timothy Chen wrote:
I think the context is that sometimes it's not desirable to remove all
orphans on recovery, especially when the discovery mechanism that a task is
launched by Mesos currently is looking for Docker containers with a mesos-
prefix (future going to be mesos-{slave_id} which is safer).
We want to leave this optionally so if users like to keep the containers
and want to have their own recovery or gc plan we can let them do so.
Btw this flag is on by default so unless users really want to it's always
killing orphan docker container tasks.
- Timothy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29328/#review68532
---
On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29328/
---
(Updated Jan. 17, 2015, 1:26 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Repository: mesos
Description
---
Add option to disable docker containerizer killing orphans
Diffs
-
src/slave/containerizer/docker.cpp 5f4b4ce
src/slave/flags.hpp a4498e6
Diff: https://reviews.apache.org/r/29328/diff/
Testing
---
make check
Thanks,
Timothy Chen
Re: Review Request 29330: Integrate docker executor into containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29330/ --- (Updated April 7, 2015, 12:39 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2595 https://issues.apache.org/jira/browse/MESOS-2595 Repository: mesos Description --- Integrate docker executor into containerizer. Diffs - src/slave/containerizer/docker.cpp f9fb078 Diff: https://reviews.apache.org/r/29330/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29329: Add executor for docker containerizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29329/ --- (Updated April 7, 2015, 12:39 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2595 https://issues.apache.org/jira/browse/MESOS-2595 Repository: mesos Description --- Add executor for docker containerizer, replaces the usage of command executor Diffs - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/docker/executor.cpp PRE-CREATION Diff: https://reviews.apache.org/r/29329/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/#review79113 --- Ship it! src/slave/paths.cpp https://reviews.apache.org/r/32911/#comment128225 Let's move the TODO into the body of the 'if' (closer to where the error checking is) and then add a comment above this 'if' that explains that we need to alays 'chown' the directory so that the permissions are correct. Feel free to go as far as noting the JIRA issue and that this had worked in the past because the fetcher was doing it but sometimes we won't be fetching anything hence the need to always do it here. Also, I'm guessing all containerizers call 'createExecutorPath' except the ExternalContainerizer? src/slave/slave.cpp https://reviews.apache.org/r/32911/#comment128223 Great comment! Can we also add something to the end of the comment that says that the user is validated when the task goes through the master? Thanks! src/slave/slave.cpp https://reviews.apache.org/r/32911/#comment128224 Why don't you need to check the taskInfo? Is it because we should have set the executorInfo's 'user' appropriately? If so, let's comment as much, even going as far as introducing a CHECK! - Benjamin Hindman On April 7, 2015, 12:40 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/ --- (Updated April 7, 2015, 12:40 a.m.) Review request for mesos, Benjamin Hindman and Ian Downes. Bugs: MESOS-2592 https://issues.apache.org/jira/browse/MESOS-2592 Repository: mesos Description --- During recent refactorings, executor directory ownership was delegated to the fetcher. However, the fetcher is not invoked if no URIs are present in the executor or task command. This left some of these tasks broken as the directory ownership defaulted to the mesos-slave's (root). Diffs - src/slave/containerizer/external_containerizer.cpp 1bbd61cb096771b7e4a1350079f79a20102e78f9 src/slave/paths.hpp 1618439d728ded347ec75317ce8dd998acd7ee94 src/slave/paths.cpp 01ea856aa2e628d4aee5fd31f7e49d147f740e8f src/slave/slave.cpp 521624c335b9110e12ee1ff21c3918e5af6a2bde Diff: https://reviews.apache.org/r/32911/diff/ Testing --- Functional tests with mesos-execute and make check. Have created JIRA's for introduction of more permission/user tests. Thanks, Niklas Nielsen
Re: Review Request 29332: Add docker_mesos_image flag to slave flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29332/ --- (Updated April 7, 2015, 12:44 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Changes --- rebased Bugs: MESOS-2183 https://issues.apache.org/jira/browse/MESOS-2183 Repository: mesos Description --- Add docker_mesos_image flag to slave flags. Diffs (updated) - src/slave/flags.hpp 3da71af Diff: https://reviews.apache.org/r/29332/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29333: Add docker_sock to slave flags
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29333/ --- (Updated April 7, 2015, 12:44 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Repository: mesos Description --- Add docker_sock to slave flags Diffs (updated) - src/slave/flags.hpp 3da71af Diff: https://reviews.apache.org/r/29333/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29327: Add slave id to docker container name prefix.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29327/ --- (Updated April 7, 2015, 12:22 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Changes --- rebased. Repository: mesos Description --- Add slave id to docker container name prefix. Diffs (updated) - src/slave/containerizer/docker.hpp 6893684e6d199a5d69fc8bba8e60c4acaae9c3c9 src/slave/containerizer/docker.cpp f9fb07806e3b7d7d2afc1be3b8756eac23b32dcd Diff: https://reviews.apache.org/r/29327/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29329: Add executor for docker containerizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29329/ --- (Updated April 7, 2015, 12:34 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2115 https://issues.apache.org/jira/browse/MESOS-2115 Repository: mesos Description --- Add executor for docker containerizer, replaces the usage of command executor Diffs (updated) - src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/docker/executor.cpp PRE-CREATION Diff: https://reviews.apache.org/r/29329/diff/ Testing --- make check Thanks, Timothy Chen
Re: Review Request 29332: Add docker_mesos_image flag to slave flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29332/ --- (Updated April 7, 2015, 12:39 a.m.) Review request for mesos, Benjamin Hindman and Bernd Mathiske. Bugs: MESOS-2183 https://issues.apache.org/jira/browse/MESOS-2183 Repository: mesos Description --- Add docker_mesos_image flag to slave flags. Diffs - src/slave/flags.hpp a4498e6 Diff: https://reviews.apache.org/r/29332/diff/ Testing --- make check Thanks, Timothy Chen
Review Request 32911: Fixed sandbox ownership bug for executors without URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/ --- Review request for mesos, Benjamin Hindman and Ian Downes. Bugs: MESOS-2592 https://issues.apache.org/jira/browse/MESOS-2592 Repository: mesos Description --- During recent refactorings, executor directory ownership was delegated to the fetcher. However, the fetcher is not invoked if no URIs are present in the executor or task command. This left some of these tasks broken as the directory ownership defaulted to the mesos-slave's (root). Diffs - src/slave/containerizer/external_containerizer.cpp 1bbd61cb096771b7e4a1350079f79a20102e78f9 src/slave/paths.hpp 1618439d728ded347ec75317ce8dd998acd7ee94 src/slave/paths.cpp 01ea856aa2e628d4aee5fd31f7e49d147f740e8f src/slave/slave.cpp 521624c335b9110e12ee1ff21c3918e5af6a2bde Diff: https://reviews.apache.org/r/32911/diff/ Testing --- Functional tests with mesos-execute and make check. Have created JIRA's for introduction of more permission/user tests. Thanks, Niklas Nielsen
Re: Review Request 32911: Fixed sandbox ownership bug for executors without URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/#review79103 --- src/slave/containerizer/external_containerizer.cpp https://reviews.apache.org/r/32911/#comment128199 Can we remove the capture by reference here? I know it's not in the style guide yet, but it will likely be accepted. - Joris Van Remoortere On April 7, 2015, 12:40 a.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/ --- (Updated April 7, 2015, 12:40 a.m.) Review request for mesos, Benjamin Hindman and Ian Downes. Bugs: MESOS-2592 https://issues.apache.org/jira/browse/MESOS-2592 Repository: mesos Description --- During recent refactorings, executor directory ownership was delegated to the fetcher. However, the fetcher is not invoked if no URIs are present in the executor or task command. This left some of these tasks broken as the directory ownership defaulted to the mesos-slave's (root). Diffs - src/slave/containerizer/external_containerizer.cpp 1bbd61cb096771b7e4a1350079f79a20102e78f9 src/slave/paths.hpp 1618439d728ded347ec75317ce8dd998acd7ee94 src/slave/paths.cpp 01ea856aa2e628d4aee5fd31f7e49d147f740e8f src/slave/slave.cpp 521624c335b9110e12ee1ff21c3918e5af6a2bde Diff: https://reviews.apache.org/r/32911/diff/ Testing --- Functional tests with mesos-execute and make check. Have created JIRA's for introduction of more permission/user tests. Thanks, Niklas Nielsen
Re: Suggestion: Mesos 0.22.1 point release
+1 - Original Message - From: Niklas Nielsen nik...@mesosphere.io To: dev dev@mesos.apache.org Sent: Friday, April 3, 2015 5:47:47 PM Subject: Re: Suggestion: Mesos 0.22.1 point release Based on input from Vinod and Adam; I will reduce the scope on the point release to focus on MESOS-1795 and MESOS-2583. I will move the other tickets back to 0.23.0 if you don't have any objections - let me know if you have any tickets which were regressions in 0.22.0. Also, this will probably generate some JIRA noise - I apologize in advance. Niklas On 3 April 2015 at 13:52, Niklas Nielsen nik...@mesosphere.io wrote: Hi everyone, I think we have everything for the point release now: https://docs.google.com/a/mesosphere.io/spreadsheets/d/1OzAWNjAL4zKtI-jOJqaQUcDNlnrNik2Dd7dHhwFLKcI/edit#gid=0 We planned on making an RC today. So with that in mind, if you have any urgent issues that needs to go into 0.22.1, please let me know :) If not, we will prepare an RC for test and baking during next week, so we can start a vote (hopefully) mid next week. I will keep you posted. Cheers, Niklas On 31 March 2015 at 11:11, Niklas Nielsen nik...@mesosphere.io wrote: Inlined On 30 March 2015 at 18:47, Dave Lester d...@davelester.org wrote: Hi Niklas, Assuming that you'd like to be release manager for this bugfix release, could you create a JIRA issue to track this and add it to the release planning wiki? https://cwiki.apache.org/confluence/display/MESOS/Mesos+Release+Planning I've already updated the page to reflect the previous release and this discussion thread but it'd be good to make this referenceable and managed in JIRA. Thanks Dave! I added the new ticket to the release wiki. I haven't investigated the particular bug you've identified, but it's worth noting that today Henning was in the IRC channel asking questions about an issue they've experienced with the latest release -- may also be worth tracking down. We are in touch and will file a ticket for the issue today. Thanks, Dave On Mon, Mar 30, 2015, at 06:32 PM, Brenden Matthews wrote: +1 for stability. On Mar 30, 2015 6:26 PM, Benjamin Hindman b...@eecs.berkeley.edu wrote: Obviously a +1, this is a stability fix we should get to our users as soon as possible. On Mon, Mar 30, 2015 at 9:01 PM, Niklas Nielsen nik...@mesosphere.io wrote: Hi all, Joris and Ben H recently located and fixed a resident bug in the state abstraction which caused many crashes in the JVM (mostly in conjunction with Marathon) at scale ( https://issues.apache.org/jira/browse/MESOS-1795) We therefore wanted to suggest doing a point release with this fix alongside any high-impact fixes which may have landed between the 0.22.0 release and now (or if reviewable and landed within a couple of days). It should be a release we can do in one to two weeks; otherwise, we should just wait for 0.23.0 Any thoughts/input? Cheers, Niklas -- Cheers, Timothy St. Clair Red Hat Inc.
Re: Review Request 32859: Add Camel-case libprocess variable and method names sample.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32859/ --- (Updated April 6, 2015, 4:36 p.m.) Review request for mesos and Niklas Nielsen. Bugs: MESOS-2576 https://issues.apache.org/jira/browse/MESOS-2576 Repository: mesos Description --- Add Camel-case libprocess variable and method names sample. Diffs (updated) - 3rdparty/libprocess/src/libev.hpp e4a403d9e769c13182f26034e0dd1c4db92b04cb 3rdparty/libprocess/src/libev.cpp 610dfb896ed8f9c00f9cf8fc8dbfc7d434f7d1e5 3rdparty/libprocess/src/libev_poll.cpp 324e4dd950989f3717ca73fe48520ca3e518518f 3rdparty/libprocess/src/process.cpp cf4e36489be2c6aa01e838c1c71383f248deab5b Diff: https://reviews.apache.org/r/32859/diff/ Testing --- Thanks, haosdent huang
