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


To fix the ExampleTests, you may want to force to use --launcher=posix in those 
tests.


src/slave/containerizer/mesos/containerizer.cpp (line 183)
<https://reviews.apache.org/r/38660/#comment157301>

    See my comments below. Let's try to simplify the logic here.



src/slave/containerizer/mesos/containerizer.cpp (lines 226 - 233)
<https://reviews.apache.org/r/38660/#comment157300>

    Hum, the logic here is confusing. We should use what the user has 
specified. If not, use linux launcher if run under root.
    
    ```
    Try<Launcher*> launcher = NULL;
    if (flags_.launcher.isSome()) {
      // If the user has specified the launcher, ...
      if (flags_.launcher.get() == "linux") {
        launcher = ...
      } else if (flags_.launcher.get() == "posix") {
        launcher = ...
      } else {
        return Error("...");
      }
    } else {
      // If the user has not specified the launcher, ...
      launcher = (::geteuid() == 0) ? ... : ...;
    }
    ```



src/slave/containerizer/mesos/containerizer.cpp (line 237)
<https://reviews.apache.org/r/38660/#comment157297>

    Should we check if the 'launcher' is not 'linux' here?



src/slave/flags.hpp (line 122)
<https://reviews.apache.org/r/38660/#comment157293>

    Please put this flag right below 'isolation'.



src/slave/flags.cpp (line 589)
<https://reviews.apache.org/r/38660/#comment157295>

    Remove the dash between Mesos and containerizer



src/slave/flags.cpp (line 591)
<https://reviews.apache.org/r/38660/#comment157294>

    s/LXC/Linux



src/slave/flags.cpp (line 592)
<https://reviews.apache.org/r/38660/#comment157296>

    s/Agent/slave
    
    Please be consistent at this moment.


- Jie Yu


On Sept. 23, 2015, 4:21 a.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38660/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3498
>     https://issues.apache.org/jira/browse/MESOS-3498
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The current logic uses LinuxLauncher if running as root. However, this causes
> problems with cgroups freezer when we launch multiple Agents inside the test
> suite.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b4a77e734706f40dc1d2110939d4f33fcbd1fd8c 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38660/diff/
> 
> 
> Testing
> -------
> 
> sudo make check currently works for all but one test. Here is the failing 
> case:
> 
> ```
> [----------] 1 test from LimitedCpuIsolatorTest
> [ RUN      ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> ../../src/tests/containerizer/isolator_tests.cpp:742: Failure
> Value of: usage.get().processes()
>   Actual: 2
> Expected: 1U
> Which is: 1
> ../../src/tests/containerizer/isolator_tests.cpp:743: Failure
> Value of: usage.get().threads()
>   Actual: 2
> Expected: 1U
> Which is: 1
> [  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids (187 ms)
> [----------] 1 test from LimitedCpuIsolatorTest (188 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (199 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
>  1 FAILED TEST
> ```
> 
> I am still investigating the failure but thought of putting out the review to 
> get some comments in the meanwhile (and some possible hints :-)).
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Reply via email to