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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Line 757 (original), 758 (patched)
<https://reviews.apache.org/r/60888/#comment265324>

    not yours, can you add a comment here saying that parent containerid will 
be guaranteed to appear first in the vector. This is important for the 
following code to always be able to find the parent container information for a 
nested container.



src/slave/paths.hpp
Lines 53-54 (patched)
<https://reviews.apache.org/r/60888/#comment265321>

    Putting this path here is a bit wierd because this directory is kind of 
containerizer specific.
    
    I'd suggest we create a `src/slave/containerizer/paths.hpp` to put those 
path helper functions.
    
    `src/slave/paths.hpp` should probably only have paths that are related to 
slave, but not containers. In the future, the sandbox for executors can be a 
symlink to the actual container sandbox.


- Jie Yu


On Oct. 16, 2017, 11:25 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60888/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 11:25 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
>     https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Although there is no way to launch standalone containers yet,
> this commit outlines the expected layout of container metadata
> which should be populated when launching standalone containers.
> 
> The layout is fairly simple, as standalone containers have no
> framework, executor, or tasks to worry about.  The sandbox directory
> will live under a new top-level directory `containers` and there is
> no metadata to checkpoint at the moment.
> 
> The containerizer will checkpoint a marker file (in the runtime
> directory) so that it knows to recover all standalone containers.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
>   src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 
>   src/slave/paths.cpp fd546525b900cb6524fb9196d19616ef18de0f30 
> 
> 
> Diff: https://reviews.apache.org/r/60888/diff/3/
> 
> 
> Testing
> -------
> 
> See next patch and later ones too.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to