You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/09/22 19:29:17 UTC
Review Request 38634: Added Systemd environment check to
LinuxLauncher.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
Bugs: MESOS-3425
https://issues.apache.org/jira/browse/MESOS-3425
Repository: mesos
Description
-------
This will be used to perform Systemd specific actions.
Diffs
-----
src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
Diff: https://reviews.apache.org/r/38634/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/
-----------------------------------------------------------
(Updated Sept. 24, 2015, 3:39 a.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
Bugs: MESOS-3425
https://issues.apache.org/jira/browse/MESOS-3425
Repository: mesos
Description
-------
This will be used to perform Systemd specific actions.
Diffs (updated)
-----
src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
src/linux/systemd.hpp PRE-CREATION
src/linux/systemd.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
Diff: https://reviews.apache.org/r/38634/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100330
-----------------------------------------------------------
Ship it!
src/linux/systemd.cpp (line 59)
<https://reviews.apache.org/r/38634/#comment157514>
Let's use tokenization and numify to avoid implicit assumptions and execeptions being thrown:
vector<string> tokens = strings::tokenize(...);
if (tokens.size() != ...) [
LOG(WARNING) << "Expecting ...";
return false;
}
string name = tokens[0];
Try<int> version = numify<int>(tokens[1]);
src/linux/systemd.cpp (line 91)
<https://reviews.apache.org/r/38634/#comment157515>
Maybe say "... your mileage may vary, but since some distributions have patched systemd pacakges available .... we're going to keep running, weeeeeee!".
- Benjamin Hindman
On Sept. 23, 2015, 10:59 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 10:59 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/
-----------------------------------------------------------
(Updated Sept. 23, 2015, 10:59 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
Bugs: MESOS-3425
https://issues.apache.org/jira/browse/MESOS-3425
Repository: mesos
Description
-------
This will be used to perform Systemd specific actions.
Diffs (updated)
-----
src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
src/linux/systemd.hpp PRE-CREATION
src/linux/systemd.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
Diff: https://reviews.apache.org/r/38634/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100310
-----------------------------------------------------------
Ship it!
Ship It!
src/linux/systemd.cpp (line 86)
<https://reviews.apache.org/r/38634/#comment157482>
'S'ee MESOS-3352.
- Timothy Chen
On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 8:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100303
-----------------------------------------------------------
src/slave/containerizer/linux_launcher.cpp (line 81)
<https://reviews.apache.org/r/38634/#comment157475>
Just wondering, why don't we rename hiearchy to freezerHierarchy here?
- Kapil Arya
On Sept. 23, 2015, 4:34 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 4:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/
-----------------------------------------------------------
(Updated Sept. 23, 2015, 8:34 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
Changes
-------
updated detection logic
Bugs: MESOS-3425
https://issues.apache.org/jira/browse/MESOS-3425
Repository: mesos
Description
-------
This will be used to perform Systemd specific actions.
Diffs (updated)
-----
src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
src/linux/systemd.hpp PRE-CREATION
src/linux/systemd.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
Diff: https://reviews.apache.org/r/38634/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Sept. 23, 2015, 8:46 a.m., Timothy Chen wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 108
> > <https://reviews.apache.org/r/38634/diff/2/?file=1081889#file1081889line108>
> >
> > Where does it create and start?
Next patch :-)
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100168
-----------------------------------------------------------
On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 8:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Sept. 23, 2015, 1:46 a.m., Timothy Chen wrote:
> > src/linux/systemd.cpp, line 54
> > <https://reviews.apache.org/r/38634/diff/2/?file=1081887#file1081887line54>
> >
> > Just tried to run this in CentOS 7 and I couldn't find /bin/systemd, but systemd is running.
I see :/ So either use the shell's PATH (again) or have fall back locations.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100168
-----------------------------------------------------------
On Sept. 22, 2015, 6:48 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 6:48 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Sept. 23, 2015, 8:46 a.m., Timothy Chen wrote:
> > src/linux/systemd.cpp, line 54
> > <https://reviews.apache.org/r/38634/diff/2/?file=1081887#file1081887line54>
> >
> > Just tried to run this in CentOS 7 and I couldn't find /bin/systemd, but systemd is running.
>
> Niklas Nielsen wrote:
> I see :/ So either use the shell's PATH (again) or have fall back locations.
Tested the new version on centos7, centos6, ubuntu15.04, ubuntu14.04, archlinux, coreos
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100168
-----------------------------------------------------------
On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 8:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100168
-----------------------------------------------------------
src/linux/systemd.cpp (line 54)
<https://reviews.apache.org/r/38634/#comment157328>
Just tried to run this in CentOS 7 and I couldn't find /bin/systemd, but systemd is running.
src/slave/containerizer/linux_launcher.cpp (line 108)
<https://reviews.apache.org/r/38634/#comment157329>
Where does it create and start?
- Timothy Chen
On Sept. 23, 2015, 1:48 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 1:48 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100155
-----------------------------------------------------------
src/linux/systemd.cpp (line 38)
<https://reviews.apache.org/r/38634/#comment157307>
Joris clarified that he is doing `ls -l` to get the real path of the file.
Wouldn't it be more appropriate to use os::realpath() here?
- Artem Harutyunyan
On Sept. 22, 2015, 6:48 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 6:48 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Sept. 23, 2015, 2:19 a.m., Artem Harutyunyan wrote:
> > src/linux/systemd.cpp, line 38
> > <https://reviews.apache.org/r/38634/diff/2/?file=1081887#file1081887line38>
> >
> > I agree with Nik, stat() would work just fine here.
using realpath.
changed the detection logic.
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100120
-----------------------------------------------------------
On Sept. 23, 2015, 8:34 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2015, 8:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am 74c1154b2a0a7a91d67be67dd32137eeaf153896
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp 459af1b98ba577863d88d521f3ba8792959f42b4
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100120
-----------------------------------------------------------
src/linux/systemd.cpp (line 38)
<https://reviews.apache.org/r/38634/#comment157251>
I agree with Nik, stat() would work just fine here.
- Artem Harutyunyan
On Sept. 22, 2015, 6:48 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 6:48 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c
> src/linux/systemd.hpp PRE-CREATION
> src/linux/systemd.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/
-----------------------------------------------------------
(Updated Sept. 23, 2015, 1:48 a.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
Changes
-------
Refactored based on BenH's comments.
Bugs: MESOS-3425
https://issues.apache.org/jira/browse/MESOS-3425
Repository: mesos
Description
-------
This will be used to perform Systemd specific actions.
Diffs (updated)
-----
src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c
src/linux/systemd.hpp PRE-CREATION
src/linux/systemd.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
Diff: https://reviews.apache.org/r/38634/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 38634: Added Systemd environment check to
LinuxLauncher.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38634/#review100111
-----------------------------------------------------------
A few questions below
src/slave/containerizer/linux_launcher.cpp (line 122)
<https://reviews.apache.org/r/38634/#comment157226>
Can you use stat()/lstat() instead? Shelling and grepping seems a bit scary
src/slave/containerizer/linux_launcher.cpp (line 138)
<https://reviews.apache.org/r/38634/#comment157227>
Full path to command and subprocess (or other execvp alternative)?
src/slave/containerizer/linux_launcher.cpp (line 165)
<https://reviews.apache.org/r/38634/#comment157229>
Move constant this to a const at the top?
Also, want to refer to a JIRA issue tracking this?
- Niklas Nielsen
On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> -----------------------------------------------------------
>
> (Updated Sept. 22, 2015, 10:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas Nielsen, Thomas Rampelberg, and Timothy Chen.
>
>
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This will be used to perform Systemd specific actions.
>
>
> Diffs
> -----
>
> src/slave/containerizer/linux_launcher.hpp f6112c98ffcc46ebcaf5581e821d5481d2f6b494
> src/slave/containerizer/linux_launcher.cpp fd0ffcf838a745ccd458d57821d358eceb85be26
>
> Diff: https://reviews.apache.org/r/38634/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>