You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2016/12/08 00:41:08 UTC
Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/
-----------------------------------------------------------
Review request for mesos and Alex Clemmer.
Bugs: MESOS-6722
https://issues.apache.org/jira/browse/MESOS-6722
Repository: mesos
Description
-------
Instead of the absolute path `/var/run/mesos`,
`os::runtime_dir()` returns a platform-specific
and permissions checked path for runtime data.
Diffs
-----
src/slave/flags.cpp 67326eece05e6300d1407ed8887aabb2f06fe9cd
Diff: https://reviews.apache.org/r/54515/diff/
Testing
-------
Thanks,
Andrew Schwartzmeyer
Re: Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/#review158508
-----------------------------------------------------------
Patch looks great!
Reviews applied: [54335, 54514, 54515]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 8, 2016, 2:03 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54515/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2016, 2:03 a.m.)
>
>
> Review request for mesos and Alex Clemmer.
>
>
> Bugs: MESOS-6722
> https://issues.apache.org/jira/browse/MESOS-6722
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of the absolute path `/var/run/mesos`,
> `os::runtime_dir()` returns a platform-specific
> and permissions checked path for runtime data.
>
> This updates the `Flags::runtime_dir` to simply default to `os::runtime_dir()`
> instead of having platform-specific code in the CLI,
> thus fully resolving MESOS-6722.
> The POSIX implementation of `os::runtime_dir()` also resolves the
> comparison of `user == "root"` as a permission check
> by correctly checking the permissions using `os::access`.
>
> This updates the `Flags::docker_volume_checkpoint_dir` to use
> `path::join(os::var(), ...)` instead of a hardcoded `/var/run/mesos` path.
>
>
> Diffs
> -----
>
> src/slave/flags.cpp 67326eece05e6300d1407ed8887aabb2f06fe9cd
>
> Diff: https://reviews.apache.org/r/54515/diff/
>
>
> Testing
> -------
>
> make && make check on Linux: no failures.
> msbuild and attach to a master on Windows: no failures.
>
> Checked that running agent as non-root on Linux *without* read/write permissions to `/var/run`
> correctly fell back to `/tmp/mesos/runtime`.
>
> Checked that running as `root` on Linux and `Administrator` on Windows
> chose the correct default `runtime_dir` paths.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/
-----------------------------------------------------------
(Updated Dec. 8, 2016, 8:14 p.m.)
Review request for mesos and Alex Clemmer.
Changes
-------
Put updated description back.
Bugs: MESOS-6722
https://issues.apache.org/jira/browse/MESOS-6722
Repository: mesos
Description (updated)
-------
Instead of the absolute path `/var/run/mesos`, `os::runtime_dir()` returns a
platform-specific and permissions checked path for runtime data.
This updates the `Flags::runtime_dir` to simply default to `os::runtime_dir()`
instead of having platform-specific code in the CLI, thus fully resolving
MESOS-6722. The POSIX implementation of `os::runtime_dir()` also resolves the
comparison of `user == "root"` as a permission check by correctly checking the
permissions using `os::access`.
This updates the `Flags::docker_volume_checkpoint_dir` to use
`path::join(os::var(), ...)` instead of a hardcoded `/var/run/mesos` path.
Diffs
-----
src/slave/flags.cpp 74a6c9936b29d35a72d47a5b7e9939c3e49fc7b3
Diff: https://reviews.apache.org/r/54515/diff/
Testing
-------
make && make check on Linux: no failures.
msbuild and attach to a master on Windows: no failures.
Checked that running agent as non-root on Linux *without* read/write permissions to `/var/run`
correctly fell back to `/tmp/mesos/runtime`.
Checked that running as `root` on Linux and `Administrator` on Windows
chose the correct default `runtime_dir` paths.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/
-----------------------------------------------------------
(Updated Dec. 8, 2016, 8:02 p.m.)
Review request for mesos and Alex Clemmer.
Bugs: MESOS-6722
https://issues.apache.org/jira/browse/MESOS-6722
Repository: mesos
Description (updated)
-------
Instead of the absolute path `/var/run/mesos`,
`os::runtime_dir()` returns a platform-specific
and permissions checked path for runtime data.
Diffs (updated)
-----
src/slave/flags.cpp 74a6c9936b29d35a72d47a5b7e9939c3e49fc7b3
Diff: https://reviews.apache.org/r/54515/diff/
Testing
-------
make && make check on Linux: no failures.
msbuild and attach to a master on Windows: no failures.
Checked that running agent as non-root on Linux *without* read/write permissions to `/var/run`
correctly fell back to `/tmp/mesos/runtime`.
Checked that running as `root` on Linux and `Administrator` on Windows
chose the correct default `runtime_dir` paths.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/#review158505
-----------------------------------------------------------
Ship it!
I'm marking this review as "Ship It" contingent on #54514 shipping. Although I do think that changes there could end up resulting in some minor changes here, it's reasonable to argue that this review will probably end up being pretty simple.
- Alex Clemmer
On Dec. 8, 2016, 2:03 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54515/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2016, 2:03 a.m.)
>
>
> Review request for mesos and Alex Clemmer.
>
>
> Bugs: MESOS-6722
> https://issues.apache.org/jira/browse/MESOS-6722
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of the absolute path `/var/run/mesos`,
> `os::runtime_dir()` returns a platform-specific
> and permissions checked path for runtime data.
>
> This updates the `Flags::runtime_dir` to simply default to `os::runtime_dir()`
> instead of having platform-specific code in the CLI,
> thus fully resolving MESOS-6722.
> The POSIX implementation of `os::runtime_dir()` also resolves the
> comparison of `user == "root"` as a permission check
> by correctly checking the permissions using `os::access`.
>
> This updates the `Flags::docker_volume_checkpoint_dir` to use
> `path::join(os::var(), ...)` instead of a hardcoded `/var/run/mesos` path.
>
>
> Diffs
> -----
>
> src/slave/flags.cpp 67326eece05e6300d1407ed8887aabb2f06fe9cd
>
> Diff: https://reviews.apache.org/r/54515/diff/
>
>
> Testing
> -------
>
> make && make check on Linux: no failures.
> msbuild and attach to a master on Windows: no failures.
>
> Checked that running agent as non-root on Linux *without* read/write permissions to `/var/run`
> correctly fell back to `/tmp/mesos/runtime`.
>
> Checked that running as `root` on Linux and `Administrator` on Windows
> chose the correct default `runtime_dir` paths.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 54515: Replace `/var/run/mesos` with
`os::runtime_dir()` in `Flags`.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54515/
-----------------------------------------------------------
(Updated Dec. 8, 2016, 2:03 a.m.)
Review request for mesos and Alex Clemmer.
Bugs: MESOS-6722
https://issues.apache.org/jira/browse/MESOS-6722
Repository: mesos
Description (updated)
-------
Instead of the absolute path `/var/run/mesos`,
`os::runtime_dir()` returns a platform-specific
and permissions checked path for runtime data.
This updates the `Flags::runtime_dir` to simply default to `os::runtime_dir()`
instead of having platform-specific code in the CLI,
thus fully resolving MESOS-6722.
The POSIX implementation of `os::runtime_dir()` also resolves the
comparison of `user == "root"` as a permission check
by correctly checking the permissions using `os::access`.
This updates the `Flags::docker_volume_checkpoint_dir` to use
`path::join(os::var(), ...)` instead of a hardcoded `/var/run/mesos` path.
Diffs
-----
src/slave/flags.cpp 67326eece05e6300d1407ed8887aabb2f06fe9cd
Diff: https://reviews.apache.org/r/54515/diff/
Testing (updated)
-------
make && make check on Linux: no failures.
msbuild and attach to a master on Windows: no failures.
Checked that running agent as non-root on Linux *without* read/write permissions to `/var/run`
correctly fell back to `/tmp/mesos/runtime`.
Checked that running as `root` on Linux and `Administrator` on Windows
chose the correct default `runtime_dir` paths.
Thanks,
Andrew Schwartzmeyer