You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2015/09/14 22:55:50 UTC
Review Request 38370: Updated docker executor to set container IP in
TaskStatus::NetworkInfo.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/
-----------------------------------------------------------
Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
Bugs: MESOS-3013
https://issues.apache.org/jira/browse/MESOS-3013
Repository: mesos
Description
-------
Updated docker executor to set container IP in TaskStatus::NetworkInfo.
Diffs
-----
src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
Diff: https://reviews.apache.org/r/38370/diff/
Testing
-------
make check.
Thanks,
Kapil Arya
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review98917
-----------------------------------------------------------
src/docker/executor.cpp (line 162)
<https://reviews.apache.org/r/38370/#comment155638>
Seems odd to have other tools referenced here in Mesos codebase. And also I think that even if Mesos-DNS migrates to use labels, are we expecting all users to force upgrade as their old version won't work with latest Mesos?
- Timothy Chen
On Sept. 14, 2015, 8:55 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2015, 8:55 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review98923
-----------------------------------------------------------
+1 on Tim's comment. I think you can just generalize the text to '3rd party tools'
Is there a corresponding test we could change to verify that this works as expected?
- Niklas Nielsen
On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2015, 1:55 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Sept. 15, 2015, 6:57 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> >
> > Reviews applied: [38363, 38364, 38365]
> >
> > Failed command: ./support/apply-review.sh -n -r 38365
> >
> > Error:
> > 2015-09-16 01:57:34 URL:https://reviews.apache.org/r/38365/diff/raw/ [16233/16233] -> "38365.patch" [1]
> > error: patch failed: src/slave/containerizer/isolators/filesystem/linux.cpp:243
> > error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does not apply
> > Failed to apply patch
Do you need to rebase?
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99149
-----------------------------------------------------------
On Sept. 15, 2015, 2:11 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 2:11 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99149
-----------------------------------------------------------
Bad patch!
Reviews applied: [38363, 38364, 38365]
Failed command: ./support/apply-review.sh -n -r 38365
Error:
2015-09-16 01:57:34 URL:https://reviews.apache.org/r/38365/diff/raw/ [16233/16233] -> "38365.patch" [1]
error: patch failed: src/slave/containerizer/isolators/filesystem/linux.cpp:243
error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Sept. 15, 2015, 9:11 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 9:11 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On Sept. 16, 2015, 3:44 p.m., Niklas Nielsen wrote:
> > src/docker/executor.cpp, lines 162-164
> > <https://reviews.apache.org/r/38370/diff/2/?file=1074454#file1074454line162>
> >
> > Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want to deprecate this over a release cycle? (Supporting both in the interim)
done!
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99284
-----------------------------------------------------------
On Sept. 16, 2015, 3:52 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 3:52 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99284
-----------------------------------------------------------
src/docker/executor.cpp (lines 162 - 164)
<https://reviews.apache.org/r/38370/#comment156192>
Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want to deprecate this over a release cycle? (Supporting both in the interim)
- Niklas Nielsen
On Sept. 15, 2015, 2:11 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 2:11 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99326
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38363, 38364, 38365, 38366, 38367, 38368, 38370]
All tests passed.
- Mesos ReviewBot
On Sept. 16, 2015, 10 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 10 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6809e4ac0018f827db27e976164c549e3a0ff823
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/
-----------------------------------------------------------
(Updated Sept. 16, 2015, 6 p.m.)
Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
Changes
-------
rebased
Bugs: MESOS-3013
https://issues.apache.org/jira/browse/MESOS-3013
Repository: mesos
Description
-------
Updated docker executor to set container IP in TaskStatus::NetworkInfo.
Diffs (updated)
-----
src/docker/executor.cpp 6809e4ac0018f827db27e976164c549e3a0ff823
src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
Diff: https://reviews.apache.org/r/38370/diff/
Testing
-------
make check.
Thanks,
Kapil Arya
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99297
-----------------------------------------------------------
Bad patch!
Reviews applied: [38363, 38364, 38365]
Failed command: ./support/apply-review.sh -n -r 38365
Error:
2015-09-16 20:48:28 URL:https://reviews.apache.org/r/38365/diff/raw/ [16233/16233] -> "38365.patch" [1]
error: patch failed: src/slave/containerizer/isolators/filesystem/linux.cpp:243
error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Sept. 16, 2015, 7:52 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:52 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/#review99289
-----------------------------------------------------------
Ship it!
Ship It!
- Niklas Nielsen
On Sept. 16, 2015, 12:52 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 12:52 p.m.)
>
>
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
>
>
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
>
>
> Diffs
> -----
>
> src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
> src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
>
> Diff: https://reviews.apache.org/r/38370/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/
-----------------------------------------------------------
(Updated Sept. 16, 2015, 3:52 p.m.)
Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
Changes
-------
addressed Nik's comment.
Bugs: MESOS-3013
https://issues.apache.org/jira/browse/MESOS-3013
Repository: mesos
Description
-------
Updated docker executor to set container IP in TaskStatus::NetworkInfo.
Diffs (updated)
-----
src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
Diff: https://reviews.apache.org/r/38370/diff/
Testing
-------
make check.
Thanks,
Kapil Arya
Re: Review Request 38370: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38370/
-----------------------------------------------------------
(Updated Sept. 15, 2015, 5:11 p.m.)
Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy Chen.
Changes
-------
addressed Tim's and Nik's comments.
Bugs: MESOS-3013
https://issues.apache.org/jira/browse/MESOS-3013
Repository: mesos
Description
-------
Updated docker executor to set container IP in TaskStatus::NetworkInfo.
Diffs (updated)
-----
src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e
src/tests/containerizer/docker_containerizer_tests.cpp a628922cc49b1dc8a37621a6a8b7ef6842cfb051
Diff: https://reviews.apache.org/r/38370/diff/
Testing
-------
make check.
Thanks,
Kapil Arya