mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 56505: Added Windows support for Docker executor.
Date Tue, 14 Feb 2017 23:24:28 GMT

-----------------------------------------------------------
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
> 
>


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