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



src/slave/containerizer/docker.cpp (line 1552)
<https://reviews.apache.org/r/36282/#comment143951>

    We have to filter even if the flag is set, since we're actually including 
more than what the flags has specified.
    
    What's interesting though is that we might unintentionally filter env 
variables if it's set from the flags and it's also included already in the os 
environment with the same key and value.
    
    I can further check with the flags.executor_environment_variables so I 
don't filter a env variable that's also set in there, but I think it might be a 
lot easier to introduce a optional boolean flag (includeOsEnvironment) on 
executorEnvironment method to be able to not include os::environment(), and 
it's set to true by default?
    
    Then all the DockerContainerizer has to do is to set that extra flag to 
false. What you think Ben?


- Timothy Chen


On July 7, 2015, 10:10 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36282/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 10:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Joerg Schad.
> 
> 
> Bugs: MESOS-2996
>     https://issues.apache.org/jira/browse/MESOS-2996
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Remove os environment when calling executorEnvironment when running docker 
> executors, since all the environment variables will be 
> passed into the container and causes bad behavior such as overriding hostname.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp 646a277be21deaded47324bea5474bd68699f25f 
>   src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba 
> 
> Diff: https://reviews.apache.org/r/36282/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Reply via email to