> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote: > > docs/configuration.md > > Lines 1329 (patched) > > <https://reviews.apache.org/r/60500/diff/1/?file=1766230#file1766230line1329> > > > > 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. > > Qian Zhang wrote: > > 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. > > I have a very old version of Docker (1.5.0) and I have checked its > `docker run` command also has the `--dns` option and it can work with > `BRIDGE` network but not with `HOST` network. Do you think we need to put > such info into this help message (I mean emphasizing the minimal Docker > version for `--dns` support with `HOST`, `BRIDGE` and `USER` networks > respectively)? That may make this message too big. > > Qian Zhang wrote: > By checking https://github.com/moby/moby/blob/master/CHANGELOG.md, I got > the following info: > 1. `--dns-opt` was introduced to `docker run` in Docker 1.9.0, see > https://github.com/moby/moby/pull/16031. > 2. `--dns` can be used with `HOST` network in Docker 1.12.0, see > https://github.com/moby/moby/pull/22408. > 3. `--dns-option` was introduced to `docker run` in Docker 1.13.0 and > `--dns-opt` was hidden (but it still can be used), see > https://github.com/moby/moby/pull/28186. > > I will do Docker version check based on the above info in > https://reviews.apache.org/r/60558/.
Sounds good. I think given the variations that docker supports with different versions I agree with you that it would be too cryptic to put this message in the help. Lets just add the `Docker version` check and be more explicit in our error messages when the version check fails. > On July 3, 2017, 5:22 p.m., Avinash sridharan wrote: > > src/messages/flags.proto > > Lines 36 (patched) > > <https://reviews.apache.org/r/60500/diff/1/?file=1766232#file1766232line36> > > > > 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. > > Qian Zhang wrote: > I think we do not need the network type since I will always use the > network name specified by framework to match `ContainerDNS.docker.network`. > And for sure, I will add some comments to explain how the network will be > matched for both CNM networks and CNI networks. Sounds good. Thanks !! - Avinash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60500/#review179520 ----------------------------------------------------------- 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 > >
