mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 50270: Introduced linux capabilities support for mesos containerizer.
Date Tue, 06 Sep 2016 18:35:43 GMT

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




src/slave/containerizer/mesos/launch.cpp (line 366)
<https://reviews.apache.org/r/50270/#comment215170>

    to avoid confusion, I'd s/capabilities/capabilitiesManager/
    
    Also, Error("Not initialized")



src/slave/containerizer/mesos/launch.cpp (line 370)
<https://reviews.apache.org/r/50270/#comment215160>

    kill this line



src/slave/containerizer/mesos/launch.cpp (line 372)
<https://reviews.apache.org/r/50270/#comment215171>

    s/instantiate/initialize/
    
    TO avoid jaggedness, i'd:
    ```
    cerr << "Failed to initialize capabilities: "
         << capabilitiesManager.error() << endl;
    ```



src/slave/containerizer/mesos/launch.cpp (line 373)
<https://reviews.apache.org/r/50270/#comment215166>

    return EXIT_FAILURE?



src/slave/containerizer/mesos/launch.cpp (line 378)
<https://reviews.apache.org/r/50270/#comment215159>

    I am wondering if we should rename this to:
    ```
    capabilities->setKeepCaps()
    capabilities->clearKeepCaps()
    ```



src/slave/containerizer/mesos/launch.cpp (line 379)
<https://reviews.apache.org/r/50270/#comment215161>

    kill this line



src/slave/containerizer/mesos/launch.cpp (lines 382 - 383)
<https://reviews.apache.org/r/50270/#comment215172>

    `uid` might be None, right?



src/slave/containerizer/mesos/launch.cpp (line 383)
<https://reviews.apache.org/r/50270/#comment215167>

    return EXIT_FAILURE?



src/slave/containerizer/mesos/launch.cpp (line 417)
<https://reviews.apache.org/r/50270/#comment215175>

    Why this is not guarded by ifdef linux?



src/slave/containerizer/mesos/launch.cpp (lines 422 - 423)
<https://reviews.apache.org/r/50270/#comment215173>

    Ditto on formatting.



src/slave/containerizer/mesos/launch.cpp (line 428)
<https://reviews.apache.org/r/50270/#comment215174>

    s/processCapabilities/capabilities/



src/slave/flags.cpp (line 94)
<https://reviews.apache.org/r/50270/#comment215147>

    This list is not complete. I'd suggest we don't make this change in this patch. It should
point to  a document online with a complete list. Let's create a ticket to track.



src/slave/flags.cpp (line 458)
<https://reviews.apache.org/r/50270/#comment215148>

    s/agent/operator/



src/slave/flags.cpp (line 460)
<https://reviews.apache.org/r/50270/#comment215149>

    s/unifed//
    
    We might want to support that for Docker containerizer in the future. You can add `(Currently
only supported in MesosContainerizer)`



src/slave/flags.cpp (line 466)
<https://reviews.apache.org/r/50270/#comment215154>

    What's this? I thought we use PR_SET_KEEPCAPS and agent is running under root.


- Jie Yu


On Sept. 6, 2016, 3:04 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50270/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5303
>     https://issues.apache.org/jira/browse/MESOS-5303
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change introduces linux capability based security for unified
> containerizer. A new agent flag `allowed_capabilities` has been
> introduced to override the default capabilities of the user or the
> capabilities requested by the user.
> 
> This feature is only available on Linux.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.hpp 0e86da9c7bd9c7fbedd7102d66b902d1c10e5e0b 
>   src/slave/containerizer/mesos/launch.cpp 13b65d82e029650e150eb2bc3647d95af167bd72 
>   src/slave/flags.hpp 1a006663e7cc58ee548b3dda686cfbac0c240baa 
>   src/slave/flags.cpp 0f2be1700f41b74da4ea1ce699a81ec33cf92a9a 
> 
> Diff: https://reviews.apache.org/r/50270/diff/
> 
> 
> Testing
> -------
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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