Re: Review Request 38900: Update command executor to support rootfs.

[Jie Yu]

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



src/launcher/executor.cpp (line 97)
<https://reviews.apache.org/r/38900/#comment158718>

    Not yours, but can you use `_override` for the parameter?



src/launcher/executor.cpp (lines 186 - 195)
<https://reviews.apache.org/r/38900/#comment158723>

    First, I would like all the preparation to be done before the fork (because 
after the fork, we technically cannot do any async signal unsafe work).
    
    Second, I don't think you need to search for the `rootVolume`. You can just 
simply assume it exists and fail if it doesn't. So the logic should be:
    ```
    Option<string> rootfs;
    
    if (sandboxDirectory is some) {
      // This is the case where the user specifies a
      // rootfs for the command.
      if (current user is not root) {
        error message
        abort();
      }
      
      if (.rootfs does not exist) {
        error message
        abort();
      }
      
      rootfs = path::join(...);
      sandbox = path::join(...);
      
      if (sandbox does not exist) {
        mkdir sandbox
      }
      
      mount the sandbox
    }
    
    ...
    fork()
    ...
    
    // In the child.
    if (rootfs.isSome()) {
      chroot(rootfs)
      chdir(sandboxDirectory)
      su(user)
    }
    ```



src/launcher/executor.cpp (lines 709 - 710)
<https://reviews.apache.org/r/38900/#comment158721>

    Please mention that these flags are only meaningful if rootfs is used for 
the user command.



src/slave/slave.cpp (lines 3227 - 3234)
<https://reviews.apache.org/r/38900/#comment158726>

    I don't think you need to do this check for mac (it's a silent ignore 
anyway). The filesystem isolator is going to reject the task during launch on 
Mac.



src/slave/slave.cpp (lines 3316 - 3325)
<https://reviews.apache.org/r/38900/#comment158727>

    Is it possible to use the non-shell version so that you don't need to worry 
about escaping?


- Jie Yu


On Oct. 2, 2015, 12:16 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38900/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2015, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3428
>     https://issues.apache.org/jira/browse/MESOS-3428
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Update command executor to support rootfs.
> 
> 
> Diffs
> -----
> 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/slave/constants.hpp df18676f17f2277e3c38432b76f16c5f9cb08341 
>   src/slave/constants.cpp cf3ee7bbc252364a1b73731feab6a9da68ee1f55 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 4c1419290645ad4c44360a81618a6cea7ad190df 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38900/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>