mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 54515: Use `os::var()` in `Flags::runtime_dir`.
Date Thu, 15 Dec 2016 23:44:02 GMT

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

(Updated Dec. 15, 2016, 11:44 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Summary (updated)
-----------------

Use `os::var()` in `Flags::runtime_dir`.


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


Repository: mesos


Description (updated)
-------

This commit improves the logic of `Flags::runtime_dir` to use
`os::var()` for encapsulation of `/var` vs `C:\ProgramData`.

However, because the suffixes are different (`/run/mesos` versus
`/mesos/runtime`), the CLI still has some platform-specific code. We
chose to keep this logic in the CLI so that it is set once, and users
can continue to consume `Flags::runtime_dir` instead of something like
`os::runtime_dir()`. This is primarily due to the fact that we have two
default paths: a system path (in `/var/run` and `ProgramData`) and a
fallback path in `/tmp` if the `os::access()` check fails. We
specifically want to avoid the situation where `os::access()` is called
multiple times due the use of `su` throughout Mesos. The `runtime_dir`
must not change after it has been set.

The permission check for the system default path is fixed to check
actual access permissions to the directory in which Mesos creates its
runtime directory, instead of assuming permissions based on a comparison
of `os::user() == "root"`.

Stylistically we chose a bit of duplication as a trade-off for enhanced
readability and auto-formatting to work properly.


Diffs (updated)
-----

  src/slave/flags.cpp a1dfbf934ef7d44cdb019aae826c90caaf477775 

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 the agent as non-root on Linux *without* read/write permissions to `/var/run`
correctly fell back to `/tmp/mesos/runtime`.

Checked that running the agent as non-administrator on Windows correctly fell back to `os::temp()/mesos/runtime`.

Checked that running as `root` on Linux and `Administrator` on Windows
chose the correct default `runtime_dir` paths.


Thanks,

Andrew Schwartzmeyer


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message