Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On Aug. 3, 2017, 2:57 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated Aug. 3, 2017, 2:57 a.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 5449b92409baa6118d1ca215f771a30bbbf9f96e 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated Aug. 3, 2017, 10:57 a.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Added more validation logic.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md 5449b92409baa6118d1ca215f771a30bbbf9f96e 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 


Diff: https://reviews.apache.org/r/60500/diff/8/

Changes: https://reviews.apache.org/r/60500/diff/7-8/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On July 26, 2017, 2:41 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 26, 2017, 2:41 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 25, 2017, 11:33 p.m., Avinash sridharan wrote:
> > src/slave/main.cpp
> > Lines 411-457 (patched)
> > 
> >
> > Shouldn't this be a \lambda?
> > 
> > Similar to how we do error handling for the flags here:
> > https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L329
> > 
> > Any specific reason we have it part of the code initialization?

My bad, I should have made it a lambda. Thanks Avinash!


- Qian


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


On July 26, 2017, 10:41 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 26, 2017, 10:41 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 26, 2017, 10:41 a.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 


Diff: https://reviews.apache.org/r/60500/diff/7/

Changes: https://reviews.apache.org/r/60500/diff/6-7/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/slave/main.cpp
Lines 411-457 (patched)


Shouldn't this be a \lambda?

Similar to how we do error handling for the flags here:
https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L329

Any specific reason we have it part of the code initialization?


- Avinash sridharan


On July 25, 2017, 6:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 25, 2017, 6:03 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 25, 2017, 2:03 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
  src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 


Diff: https://reviews.apache.org/r/60500/diff/6/

Changes: https://reviews.apache.org/r/60500/diff/5-6/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 25, 2017, 1:46 a.m., Jie Yu wrote:
> > src/messages/flags.proto
> > Lines 41 (patched)
> > 
> >
> > I'd s/MesosDNS/MesosInfo/ here. Same for DockerDNS below. This seems to 
> > be consistent with what's inside ContainerInfo.

Agree, and I will do s/ContainerDNS/ContainerDNSInfo/ as well.


- Qian


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


On July 24, 2017, 9:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 24, 2017, 9:39 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 25, 2017, 1:42 a.m., Avinash sridharan wrote:
> > src/slave/flags.cpp
> > Lines 773 (patched)
> > 
> >
> > We need a validation for the `HOST` mode here?

Yeah, I have it here:
https://reviews.apache.org/r/60500/diff/5#5


- Qian


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


On July 24, 2017, 9:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 24, 2017, 9:39 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Jie Yu]

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


Fix it, then Ship it!




The protobuf looks good to me! Thanks Qian!


src/messages/flags.proto
Lines 41 (patched)


I'd s/MesosDNS/MesosInfo/ here. Same for DockerDNS below. This seems to be 
consistent with what's inside ContainerInfo.


- Jie Yu


On July 24, 2017, 1:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 24, 2017, 1:39 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/slave/flags.cpp
Lines 773 (patched)


We need a validation for the `HOST` mode here?


- Avinash sridharan


On July 24, 2017, 1:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 24, 2017, 1:39 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

> On July 23, 2017, 3:22 p.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 69 (patched)
> > 
> >
> > Qian, sorry, should have pointed this out earlier. I think we decided 
> > that for Docker we can support HOST mode as well? So we can remove this 
> > comment.
> 
> Qian Zhang wrote:
> I remember that we decided to support BRIDGE mode for Docker, but not 
> support HOST mode for both CNI and Docker so that they can have a consistent 
> behavior regarding HOST mode. Can you please double confirm with Jie?

Ah ok!! I am fine with not supporting HOST mode for docker. The use-case isn't 
that strong. Probably the confusion arose on my part because your patches 
already did support HOST mode for Docker so though we were planning to keep 
that support. Lets drop this in that case and explicitly not support HOST mode 
for docker.


- Avinash


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


On July 24, 2017, 1:39 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 24, 2017, 1:39 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 24, 2017, 9:39 a.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
  src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 


Diff: https://reviews.apache.org/r/60500/diff/5/

Changes: https://reviews.apache.org/r/60500/diff/4-5/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 23, 2017, 11:22 p.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 69 (patched)
> > 
> >
> > Qian, sorry, should have pointed this out earlier. I think we decided 
> > that for Docker we can support HOST mode as well? So we can remove this 
> > comment.

I remember that we decided to support BRIDGE mode for Docker, but not support 
HOST mode for both CNI and Docker so that they can have a consistent behavior 
regarding HOST mode. Can you please double confirm with Jie?


> On July 23, 2017, 11:22 p.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 90 (patched)
> > 
> >
> > Seems a bit odd that we are re-using CNI over here for the DNS 
> > semantics. I am inclined to define a CNM/DNS protobuf that we can re-use 
> > here, following the semantics of keeping these pieces completely segregated?

Agree!


- Qian


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


On July 23, 2017, 5:29 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 23, 2017, 5:29 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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.proto
Lines 69 (patched)


Qian, sorry, should have pointed this out earlier. I think we decided that 
for Docker we can support HOST mode as well? So we can remove this comment.



src/messages/flags.proto
Lines 90 (patched)


Seems a bit odd that we are re-using CNI over here for the DNS semantics. I 
am inclined to define a CNM/DNS protobuf that we can re-use here, following the 
semantics of keeping these pieces completely segregated?


- Avinash sridharan


On July 23, 2017, 9:29 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 23, 2017, 9:29 a.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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 22, 2017, 12:34 a.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 44 (patched)
> > 
> >
> > I know we are using this `USER` mode from the docker terminology, but I 
> > would really like to move away from that terminology if possible and be 
> > explicit here that this is a CNI network? Instead of calling this mode 
> > `USER` can we just call it `CNI?

Good point!


> On July 22, 2017, 12:34 a.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 49-54 (patched)
> > 
> >
> > Can we change this to:
> > 
> > This field is valid only when the `network_mode` is set to `CNI`. It is 
> > invalid when the `network_mode` is `HOST`. When the mode is `CNI` this 
> > field informs the `network/cni` isolator about the `CNI` network to which 
> > the DNS configuration applies. Also, if the mode is `CNI` and this field is 
> > not set it implies wild card semantics for the CNI network name, informing 
> > the `network/cni` isolator that the DNS configuration applies to "all" CNI 
> > networks. 
> > 
> > NOTE: If there are multiple `MesosDNS` configuration specified, when 
> > applyig a DNS configuration to a CNI network the container will always 
> > choose the most specific match based on the CNI network name.

Agree!


- Qian


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


On July 23, 2017, 5:29 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 23, 2017, 5:29 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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 23, 2017, 5:29 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
  src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 


Diff: https://reviews.apache.org/r/60500/diff/4/

Changes: https://reviews.apache.org/r/60500/diff/3-4/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.proto
Lines 41 (patched)


Can we do `MesosDNS`? Following the cadence of `ContainerInfo` and 
`MesosInfo`, this would be `ContainerDNS` and `MesosDNS`?



src/messages/flags.proto
Lines 43 (patched)


s/Not supported yet/Currently not supported.



src/messages/flags.proto
Lines 44 (patched)


I know we are using this `USER` mode from the docker terminology, but I 
would really like to move away from that terminology if possible and be 
explicit here that this is a CNI network? Instead of calling this mode `USER` 
can we just call it `CNI?



src/messages/flags.proto
Lines 49-54 (patched)


Can we change this to:

This field is valid only when the `network_mode` is set to `CNI`. It is 
invalid when the `network_mode` is `HOST`. When the mode is `CNI` this field 
informs the `network/cni` isolator about the `CNI` network to which the DNS 
configuration applies. Also, if the mode is `CNI` and this field is not set it 
implies wild card semantics for the CNI network name, informing the 
`network/cni` isolator that the DNS configuration applies to "all" CNI 
networks. 

NOTE: If there are multiple `MesosDNS` configuration specified, when 
applyig a DNS configuration to a CNI network the container will always choose 
the most specific match based on the CNI network name.



src/messages/flags.proto
Lines 63 (patched)


s/Not supported yet/Currently not supported.



src/messages/flags.proto
Lines 70-75 (patched)


This field is valid only when the mode is `USER`. For `USER` mode this 
specified the CNM network to which this DNS configuration applies. Also, if the 
mode is `USER` and this field is not set it implies wild card semantics for CNM 
networks, i.e, this DNS configuration applies to all CNM networks.

NOTE: In case there are multiple `DockerDNS` specified, when appliying the 
DNS configuration to a CNM network the containerizer will always choose the 
most specific match.


- Avinash sridharan


On July 20, 2017, 12:21 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 20, 2017, 12:21 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 a06ac82e7b908601b9ba97011931276e6292102d 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
>   src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 20, 2017, 8:21 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 


Diff: https://reviews.apache.org/r/60500/diff/3/

Changes: https://reviews.apache.org/r/60500/diff/2-3/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

> On July 17, 2017, 11:12 p.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 56 (patched)
> > 
> >
> > Had an internal discussion on this with Jie, and seems like having 
> > different DNS options for CNI and CNM is not something we need to worry 
> > about, so we should merge the fields into one.
> 
> Qian Zhang wrote:
> OK, so the protobuf message should be like the below, right?
> ```
> message ContainerDNS {
>   message DNSInfo {
> // Specify CNI network name or CNM network name as the value of
> // of this field. For CNM host network, its name is `host`, for
> // CNM default bridge network, its name is `bridge`, for a CNM
> // user-defined network, its name is specified by:
> // `ContainerInfo.network_infos(0).name`.
> required string network = 1;
> 
> // For CNI network, all four fields in `slave.cni.spec.DNS` are
> // supported, but for CNM network, we only support three fields:
> // `nameservers`, `search` and `options` but not `domain` since
> // Docker only has `--dns`, `--dns-search`, `--dns-option` options.
> required slave.cni.spec.DNS dns = 2;
>   }
> 
>   repeated DNSInfo dns = 1;
> }
> ```

That looks about right. Another thing that came out of the discussion is that 
we should probably make the `network` field optional. This would then follow 
the semantic of the `network/cni` isolator for missing `network` field implying 
host networks. That said, currently, we should not be supporting setting DNS 
for the host networking since the behavior is different for CNI and CNM. Also, 
changing DNS for host networking doesn't make that much sense. To protect 
against host networking we can add a validation check as a lambda in the 
`--default_container_dns` flag? 

For any unsupported DNS options (`domain` in CNM) we should throw a 
`LOG(WARNING)` while configuring the container's DNS.


- Avinash


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


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 18, 2017, 7:12 a.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 56 (patched)
> > 
> >
> > Had an internal discussion on this with Jie, and seems like having 
> > different DNS options for CNI and CNM is not something we need to worry 
> > about, so we should merge the fields into one.

OK, so the protobuf message should be like the below, right?
```
message ContainerDNS {
  message DNSInfo {
// Specify CNI network name or CNM network name as the value of
// of this field. For CNM host network, its name is `host`, for
// CNM default bridge network, its name is `bridge`, for a CNM
// user-defined network, its name is specified by:
// `ContainerInfo.network_infos(0).name`.
required string network = 1;

// For CNI network, all four fields in `slave.cni.spec.DNS` are
// supported, but for CNM network, we only support three fields:
// `nameservers`, `search` and `options` but not `domain` since
// Docker only has `--dns`, `--dns-search`, `--dns-option` options.
required slave.cni.spec.DNS dns = 2;
  }

  repeated DNSInfo dns = 1;
}
```


- Qian


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


On July 5, 2017, 3:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 3:03 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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.proto
Lines 56 (patched)


Had an internal discussion on this with Jie, and seems like having 
different DNS options for CNI and CNM is not something we need to worry about, 
so we should merge the fields into one.


- Avinash sridharan


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.proto
Lines 56 (patched)


I see Jie's point. From a framework's standpoint a name is a single domain 
of IP connectivity, so ideally if we have a network name that is both a CNI and 
CNM network (DC/OS overlay or Calico comes to mind here), it implies that it 
should be a single network and hence having a common DNS configuration across 
CNI/CNM is not incorrect. 

That said instead of using Mesos and Docker as the fields if we use CNI and 
CNM as the field names would that make sense? We would then be pegging against 
the standard rather than the `Containerizer`? The reason I am proposing to keep 
two different fields here is primarily to take into consideration any 
incosistencies that might pop up in these standards at some point (right now 
there don't seem to be any for DNS).


- Avinash sridharan


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Jie Yu]

> On July 14, 2017, 6:31 p.m., Jie Yu wrote:
> > src/messages/flags.proto
> > Lines 56 (patched)
> > 
> >
> > Any reason we seperate 'mesos' from 'docker'? Can we use the same?
> 
> Qian Zhang wrote:
> The reason that we have 'mesos' and 'docker' here is that we want 
> operator to be able to set DNS info for CNI networks and CNM networks 
> separately. Jie, did you mean we may need to merge them into one like below 
> so that we do not have 'mesos' and 'docker' anymore?
> ```
> {
>   [
> {
>   "network": "bridge",
>   "dns": {
> "nameservers": [ "8.8.8.8" ]
>   }
> },
> {
>   "network": "net1",
>   "dns": {
> "nameservers": [ "8.8.4.4" ]
>   }
> }
>   ]
> }
> ```
> I think it should be OK. The only potential issue for the above design in 
> my mind is, if there are 2 networks, 1 CNI and 1 CNM, but they have the same 
> name (say `net1`), and operator may want to set different DNS for them 
> respectively which can not be achieved with the above design.

The reason I am asking is that: we should ultimately avoid having flags that 
are specific to either Docker/Mesos containerizer. We should ask the 
containerizer to provide consistent behaviors. It looks to me that "default DNS 
for the container" should apply to both containerizers.


- Jie


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


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 15, 2017, 2:31 a.m., Jie Yu wrote:
> > src/messages/flags.proto
> > Lines 56 (patched)
> > 
> >
> > Any reason we seperate 'mesos' from 'docker'? Can we use the same?

The reason that we have 'mesos' and 'docker' here is that we want operator to 
be able to set DNS info for CNI networks and CNM networks separately. Jie, did 
you mean we may need to merge them into one like below so that we do not have 
'mesos' and 'docker' anymore?
```
{
  [
{
  "network": "bridge",
  "dns": {
"nameservers": [ "8.8.8.8" ]
  }
},
{
  "network": "net1",
  "dns": {
"nameservers": [ "8.8.4.4" ]
  }
}
  ]
}
```
I think it should be OK. The only potential issue for the above design in my 
mind is, if there are 2 networks, 1 CNI and 1 CNM, but they have the same name 
(say `net1`), and operator may want to set different DNS for them respectively 
which can not be achieved with the above design.


- Qian


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


On July 5, 2017, 3:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 3:03 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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Jie Yu]

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




src/messages/flags.proto
Lines 56 (patched)


Any reason we seperate 'mesos' from 'docker'? Can we use the same?


- Jie Yu


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 4, 2017, 1:22 a.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1312 (patched)
> > 
> >
> > `ContainerInfo.docker.parameter`
> 
> Qian Zhang wrote:
> The field name is `parameters` rather than `parameter`.
> 
> Avinash sridharan wrote:
> Qian I meant put the field in quotest `ContainerInfo.docker.parameters`.

Avinash, did you mean `ContainerInfo.docker.parameters`? That will make the 
line in flags.cpp exceeding 80 chars, and when you look at the flag 
`default_container_info`, in its help message, the `ContainerInfo` is not in 
quotest too.


- Qian


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


On July 5, 2017, 3:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 3:03 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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1312 (patched)
> > 
> >
> > `ContainerInfo.docker.parameter`
> 
> Qian Zhang wrote:
> The field name is `parameters` rather than `parameter`.

Qian I meant put the field in quotest `ContainerInfo.docker.parameters`.


- Avinash


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


On July 5, 2017, 7:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated July 5, 2017, 7:03 a.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 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 5, 2017, 3:03 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed Avinash's comments.


Bugs: MESOS-7709
https://issues.apache.org/jira/browse/MESOS-7709


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
  src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 


Diff: https://reviews.apache.org/r/60500/diff/2/

Changes: https://reviews.apache.org/r/60500/diff/1-2/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1329 (patched)
> > 
> >
> > 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)
> > 
> >
> > 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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 4, 2017, 1:22 a.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1329 (patched)
> > 
> >
> > 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.

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


- Qian


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


On June 28, 2017, 10: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, 10: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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

> On July 4, 2017, 1:22 a.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1312 (patched)
> > 
> >
> > `ContainerInfo.docker.parameter`

The field name is `parameters` rather than `parameter`.


> On July 4, 2017, 1:22 a.m., Avinash sridharan wrote:
> > docs/configuration.md
> > Lines 1329 (patched)
> > 
> >
> > 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.

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


> On July 4, 2017, 1:22 a.m., Avinash sridharan wrote:
> > src/messages/flags.proto
> > Lines 36 (patched)
> > 
> >
> > 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.

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.


- Qian


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


On June 28, 2017, 10: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, 10: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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.hpp
Lines 58 (patched)


For CNI, since we are adding these entries to the `resolv.conf` we can't 
support more than three name servers for a given network.

Similarly for CNM we probably need to check if we can support more than 3 
nameservers for HOST and BRIDGE networks?


- 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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




docs/configuration.md
Lines 1309 (patched)


s/CNI plugin/ a CNI plugin,



docs/configuration.md
Lines 1312 (patched)


`ContainerInfo.docker.parameter`



docs/configuration.md
Lines 1329 (patched)


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)


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)


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



src/messages/flags.proto
Lines 36 (patched)


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)


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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60500]

Failed command: python support/apply-reviews.py -n -r 60500

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 194, in fetch_patch
patch.write(patch.read())
IOError: File not open for reading

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/124/console

- Mesos Reviewbot Windows


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 c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

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 c84aa6724170bba46bbe8410b71d42a1626e 


Diff: https://reviews.apache.org/r/60500/diff/1/


Testing
---


Thanks,

Qian Zhang