----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53699/#review156772 -----------------------------------------------------------
Fix it, then Ship it! Modulo a couple of things I'll tweak before committing (see comments). src/tests/container_logger_tests.cpp (line 93) <https://reviews.apache.org/r/53699/#comment227049> The leading `::` is not needed here. It looks like we aren't 100% consistent about this in the codebase. src/tests/container_logger_tests.cpp (lines 585 - 612) <https://reviews.apache.org/r/53699/#comment227051> Benjamin made a comment in the original review (https://reviews.apache.org/r/52059/) about changing this user to "nobody". We can simplify the code a bit if we do so. src/tests/container_logger_tests.cpp (lines 625 - 629) <https://reviews.apache.org/r/53699/#comment227052> This comment doesn't quite match the test contents... ``` // 1. When `--switch_user` is true on the agent, the logger module should // launch subprocesses with the same user as the executor. // 2. When `--switch_user` is false on the agent, the logger module should // inherit the user of the agent. ``` src/tests/container_logger_tests.cpp (line 771) <https://reviews.apache.org/r/53699/#comment227053> Don't need an additional comparison here. `GetParam()` is already a boolean. - Joseph Wu On Nov. 17, 2016, 12:42 p.m., Sivaram Kannan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53699/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2016, 12:42 p.m.) > > > Review request for mesos and Joseph Wu. > > > Bugs: MESOS-5856 > https://issues.apache.org/jira/browse/MESOS-5856 > > > Repository: mesos > > > Description > ------- > > Add test cases to test logrotate with switch_user set to true and false. > > > Diffs > ----- > > src/tests/container_logger_tests.cpp > 1bb94a8461e481983f25a44737e4011ed5fc4b1f > > Diff: https://reviews.apache.org/r/53699/diff/ > > > Testing > ------- > > Run the mesos-logrotate-logger with un-priviledged user and verify whether > the logs are getting rotated. > Run the mesos-logrotate-logger as root user and verify whether the logs are > getting rotated. > > > Thanks, > > Sivaram Kannan > >
