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 56341: Changed docker/runtime isolator's handling of Environment.
Date Tue, 07 Feb 2017 19:17:44 GMT

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




src/launcher/executor.cpp (line 860)
<https://reviews.apache.org/r/56341/#comment236297>

    Similar to capabilities, can we add a parse function for Environment and use `Option<Environment>`
here.



src/launcher/executor.cpp (line 928)
<https://reviews.apache.org/r/56341/#comment236298>

    Let's avoid the `mesos::` prefix here by using a `using` clause at the begining of the
file.



src/launcher/executor.cpp (lines 937 - 940)
<https://reviews.apache.org/r/56341/#comment236299>

    Let's just pass `environment` to `CommandExecutor` below.



src/launcher/executor.cpp (line 939)
<https://reviews.apache.org/r/56341/#comment236300>

    I'd do that in `launch`.
    
    I'll also add a TODO. Ideally, we should use execvpe, rather than relying on inheritence.
    
    Are you sure it won't affect command health check if you do that in this way?



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 140)
<https://reviews.apache.org/r/56341/#comment236303>

    Let's update AppcRuntimeIsolator as well.


- Jie Yu


On Feb. 7, 2017, 1:19 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56341/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 1:19 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-7027
>     https://issues.apache.org/jira/browse/MESOS-7027
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit adds more special-casing in the `docker/runtime` isolator
> for the command executor.  The command executor will generally break
> when the `docker/runtime` isolator provides environment variables
> directly to the executor.  This is because the environment variables
> are provided in the context of the container image, rather than the
> host.
> 
> For example, a container image may provide an environment variable
> like `LD_LIBRARY_PATH=/image/specific/location`, whereas the default
> executor expects to find libraries in the host's environment.  If the
> image's environment end up in the command executor at launch time, 
> the command executor may simply fail to launch.
> 
> 
> Diffs
> -----
> 
>   src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 2d816e512c95ed2922c9578ba796908c5ce23da4

> 
> Diff: https://reviews.apache.org/r/56341/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo src/mesos-tests --gtest_filter="*ROOT*"
> 
> All the tests broken by https://reviews.apache.org/r/56339/ (earlier in the chain) are
now passing.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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