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




src/docker/docker.hpp (lines 41 - 47)
<https://reviews.apache.org/r/56505/#comment237442>

    This isn't a default, as you can't change the value without breaking it.



src/docker/executor.hpp (lines 51 - 52)
<https://reviews.apache.org/r/56505/#comment237443>

    That last note can be ommitted, as this argument is supplied by the agent, 
even if the Mesos agent is not running in Docker (recommended).  Also, if you 
were to invoke the docker executor in a stand-alone way, this note does not 
make sense.



src/slave/constants.hpp (lines 101 - 102)
<https://reviews.apache.org/r/56505/#comment237444>

    Copy-pasted comment :)
    
    Suggestion:
    Default UNIX socket (Linux) or Named Pipe (Windows) resource that provides 
CLI access to the Docker daemon.



src/slave/containerizer/docker.cpp (lines 1408 - 1420)
<https://reviews.apache.org/r/56505/#comment237452>

    I would like to see this block and the same block in `mesos/launch.cpp` be 
separated into another review.
    
    The pipe naming changes and this environment change (technically a no-op 
because Subprocess on Windows is already enforcing it) are not highly 
inter-related.



src/slave/containerizer/mesos/launch.cpp (lines 101 - 103)
<https://reviews.apache.org/r/56505/#comment237450>

    Why is this removed from the `#ifdef`?  It still isn't used anywhere on 
Windows.


- Joseph Wu


On Feb. 14, 2017, 11:06 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56505/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2017, 11:06 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-3098
>     https://issues.apache.org/jira/browse/MESOS-3098
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit will introduce the changes to the Mesos Docker interfaces
> required to use the Docker executor to run Windows Containers in the
> Mesos agent. In particular, since Windows Containers use named pipes to
> connect do the Docker host, rather than a socket, we introduce the
> plumbing to default to a named pipe connection when invoking the
> `docker` command.
> 
> This work constitutes the beginning of the end of the work that will
> eventually result in Mesos support of Windows Containers.
> 
> This review is partially based on r/52364, which was the work of Daniel
> Pravat.
> 
> Lastly, this review has a planned regression in MESOS-6816. In other
> parts of the codebase, we copy all environment variables from the system
> before launching a process, overwriting user specified environment
> variables. This is not correct behavior, but a half-measure that is
> necessary, because Windows does not inherit environment variables by
> default. In this commit, we make this behavior uniform across all places
> where we are creating a process, because it is better for behavior to be
> consistent before we get around to fixing it. We do have concrete plans
> to come back and resolve this issue.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 68042fd6f3b19c12cddd0d2e74ae064ceceb6a8c 
>   src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
>   src/slave/constants.hpp 1159ac3b1f8797a847a6f5679045434064a519e9 
>   src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 
>   src/slave/containerizer/mesos/launch.hpp 
> 49eb9128ae5a1e4ec02e2b9d9e3cb67d7a8f7663 
>   src/slave/containerizer/mesos/launch.cpp 
> 4dd81b47ca4654f5e783a4f2227834e938bc8bb3 
>   src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606 
>   src/tests/flags.hpp cab24162e26be9fe79c0c65f24e35e3c3d735906 
> 
> Diff: https://reviews.apache.org/r/56505/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Reply via email to