mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 70011: Modified `LOGROTATE_CustomRotateOptions` test after memfd change.
Date Tue, 19 Feb 2019 20:02:50 GMT

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


Fix it, then Ship it!




Looks good, with some small suggestions:


src/tests/container_logger_tests.cpp
Lines 423-425 (original), 424-427 (patched)
<https://reviews.apache.org/r/70011/#comment298797>

    You could use the test directory here:
    ```
    const string testFile = sandbox.get() + "/CustomRotateOptions";
    ```



src/tests/container_logger_tests.cpp
Lines 469-471 (original), 493-495 (patched)
<https://reviews.apache.org/r/70011/#comment298796>

    It may be worthwhile to check for the absence of the config file when launcher sealing
is enabled, since the module has the fallback behavior of writing the config file.  We want
to make sure the fallback is not being used when the build is configured properly.
    ```
    #ifdef ENABLE_LAUNCHER_SEALING
    ASSERT_FALSE(os::exists(stdoutPath));
    #endif // ENABLE_LAUNCHER_SEALING
    ```


- Joseph Wu


On Feb. 19, 2019, 9:06 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70011/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2019, 9:06 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9564
>     https://issues.apache.org/jira/browse/MESOS-9564
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch changes the mechanism of verifying application of custom
> logrotation options. Since we don't create logrotate config file
> in the container's sandbox on Linux, we can't use the previous
> approach where we tried to find the custom options in the generated
> `stdout.logrotate.conf`. Instead, we inject a script into custom
> options, which creates an empty file in the temporary directory on
> log rotation.
> 
> 
> Diffs
> -----
> 
>   src/tests/container_logger_tests.cpp 6c6b3152d82a3c9825a0b4f1b5f8c9a065cb2927 
> 
> 
> Diff: https://reviews.apache.org/r/70011/diff/1/
> 
> 
> Testing
> -------
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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