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




src/slave/validation.cpp
Line 395 (original), 406 (patched)
<https://reviews.apache.org/r/72221/#comment308169>

    Kill this line.



src/slave/validation.cpp
Lines 457 (patched)
<https://reviews.apache.org/r/72221/#comment308170>

    Need to check `launch.has_container()` first?



src/slave/validation.cpp
Lines 460-465 (patched)
<https://reviews.apache.org/r/72221/#comment308173>

    Why do we need this check?



src/slave/validation.cpp
Lines 467-472 (patched)
<https://reviews.apache.org/r/72221/#comment308172>

    I think here what you check is the resource requests, but we also need to 
ensure no resource limits specified for such containers, and also mention in 
the error message that such containers cannot have `share_cgroup` as false.



src/slave/validation.cpp
Lines 473-479 (patched)
<https://reviews.apache.org/r/72221/#comment308175>

    Why do we need this check? I think we should allow launching 1st level 
nested container with resource requests specified and `share_cgroups` as true, 
right? That's actually what the default executor does in our current 
implementation.



src/slave/validation.cpp
Lines 482 (patched)
<https://reviews.apache.org/r/72221/#comment308174>

    Need to check `launch.has_container()` first?


- Qian Zhang


On March 13, 2020, 12:12 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72221/
> -----------------------------------------------------------
> 
> (Updated March 13, 2020, 12:12 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Qian Zhang.
> 
> 
> Bugs: MESOS-10045
>     https://issues.apache.org/jira/browse/MESOS-10045
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added agent validation for shared cgroups.
> 
> 
> Diffs
> -----
> 
>   src/slave/validation.cpp 99b17c965d4cc522e9106078a9e6f34fae396b8a 
> 
> 
> Diff: https://reviews.apache.org/r/72221/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to