mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Avinash sridharan <avin...@mesosphere.io>
Subject Re: Review Request 60500: Introduced `--default_container_dns` agent flag.
Date Mon, 03 Jul 2017 17:22:52 GMT

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




docs/configuration.md
Lines 1309 (patched)
<https://reviews.apache.org/r/60500/#comment254266>

    s/CNI plugin/ a CNI plugin,



docs/configuration.md
Lines 1312 (patched)
<https://reviews.apache.org/r/60500/#comment254267>

    `ContainerInfo.docker.parameter`



docs/configuration.md
Lines 1329 (patched)
<https://reviews.apache.org/r/60500/#comment254268>

    This is interesting. I think this example highlights a subtle difference between the behavior
of the DNS option for CNI and CNM. For CNI networks we don't really have a concept of HOST
or BRIDGE networks. However, for CNM networks since we support this option for HOST, BRIDGE
and USER networks, we should be explicit about this information. Also, we need to emphasize
that the `--dns` options for `HOST` and `BRIDGE` networks are supported only for docker 1.13
and above (for `USER` it has been available since 1.10). We probably want to enforce these
version checks for docker.



src/messages/flags.proto
Line 21 (original), 23 (patched)
<https://reviews.apache.org/r/60500/#comment254273>

    Not yours but might as well fix the comment formatting to conform with the rest of the
`.proto` files?



src/messages/flags.proto
Lines 32 (patched)
<https://reviews.apache.org/r/60500/#comment254271>

    2 lines apart? Also, I think for protobuf definitions we are using the following format
for comments:
    /**
     * ....
     */



src/messages/flags.proto
Lines 36 (patched)
<https://reviews.apache.org/r/60500/#comment254275>

    Just a thought, should we be more explicit and keep two different DNS structures for Mesos
and Docker? For example, Docker has the concept of network modes where as Mesos doesn't. With
this structure we would have to use some kind of pattern matching to identify the network
type in case of docker, e.g., `network="host"` for host-mode networking, `network="bridge"`
for bridge-mode networking. This is just a thought, I am fine with these validation checks
being introduced if this seems cleaner.
    
    At the very least we should definitely have a comment here explaining how the network-modes
are selected for `docker` based on the network names.



src/slave/flags.cpp
Lines 766 (patched)
<https://reviews.apache.org/r/60500/#comment254277>

    Ditto for comments on `ocnfigure.md`.


- Avinash sridharan


On June 28, 2017, 2:57 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> -----------------------------------------------------------
> 
> (Updated June 28, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
>     https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduced `--default_container_dns` agent flag.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46b4444be8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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