You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2014/08/08 00:01:37 UTC
Review Request 24475: Add new Docker configurations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs
-----
include/mesos/mesos.proto efb4239
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 5:08 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 844
> > <https://reviews.apache.org/r/24475/diff/2/?file=657105#file657105line844>
> >
> > I'm not sure what the practice is, but thought it's easier to reserve 1 - 100 for common properties and 100 - max for union types. I can just number them serially but thought it can get confusing since we don't want to order the fields by number but by sections. What you think?
>
> Benjamin Hindman wrote:
> Folks usually just look for the highest number and add 1. If they see 100 they're not going to know that N - 100 are actually "available" unless that's documented since it's possible that a field was added, deprecated, and then removed. This is also why some people keep all deprecated fields but just move them to the bottom, so that field numbers are guaranteed not to be reused!
Sounds good, I'll change this.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50149
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Aug. 11, 2014, 5:08 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 844
> > <https://reviews.apache.org/r/24475/diff/2/?file=657105#file657105line844>
> >
> > I'm not sure what the practice is, but thought it's easier to reserve 1 - 100 for common properties and 100 - max for union types. I can just number them serially but thought it can get confusing since we don't want to order the fields by number but by sections. What you think?
Folks usually just look for the highest number and add 1. If they see 100 they're not going to know that N - 100 are actually "available" unless that's documented since it's possible that a field was added, deprecated, and then removed. This is also why some people keep all deprecated fields but just move them to the bottom, so that field numbers are guaranteed not to be reused!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50149
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50149
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/24475/#comment87745>
I'm not sure what the practice is, but thought it's easier to reserve 1 - 100 for common properties and 100 - max for union types. I can just number them serially but thought it can get confusing since we don't want to order the fields by number but by sections. What you think?
- Timothy Chen
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50347
-----------------------------------------------------------
flying by review:)
src/docker/docker.hpp
<https://reviews.apache.org/r/24475/#comment88038>
Given that, consider putting docker.hpp/cpp into namespace mesos::internal ?
- Jie Yu
On Aug. 12, 2014, 8:45 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2014, 8:45 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50404
-----------------------------------------------------------
Ship it!
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment88179>
What about taking a lambda::_1 here to just get the failure message? Then you can pass just failure messages to _fetchFailed from _fetch without having to pass the whole future.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment88178>
A CHECK_FAILED(status) would be nice here, but I don't think it's necessary if this function just takes a string.
- Benjamin Hindman
On Aug. 13, 2014, 2:59 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 2:59 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 14, 2014, 1:37 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Changes
-------
Took out docker logs from this reviewboard and addressed comments.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs (updated)
-----
include/mesos/mesos.proto adc8fab
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/flags.hpp 1e36c51
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 13, 2014, 8:36 p.m., Ian Downes wrote:
> > src/tests/docker_containerizer_tests.cpp, line 337
> > <https://reviews.apache.org/r/24475/diff/5/?file=659137#file659137line337>
> >
> > Why the change in sleep time? Why not "sleep 1000" which is used elsewhere for a "long" enough sleep.
The sleep time differs as we originally use a large image to test launching executor.
I agree we can just a consistent time now
> On Aug. 13, 2014, 8:36 p.m., Ian Downes wrote:
> > src/slave/containerizer/docker.cpp, line 852
> > <https://reviews.apache.org/r/24475/diff/5/?file=659133#file659133line852>
> >
> > Sorry if I missed it, but how does logging redirection get resumed when the slave restarts?
It's not handled yet, and Ben and I are working on that
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50485
-----------------------------------------------------------
On Aug. 13, 2014, 5:47 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 5:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 13, 2014, 8:36 p.m., Ian Downes wrote:
> > src/docker/docker.cpp, line 303
> > <https://reviews.apache.org/r/24475/diff/5/?file=659130#file659130line303>
> >
> > s/to//
I hope I can grok grammer one day.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50485
-----------------------------------------------------------
On Aug. 13, 2014, 5:47 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 5:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 13, 2014, 8:36 p.m., Ian Downes wrote:
> > src/tests/docker_containerizer_tests.cpp, line 337
> > <https://reviews.apache.org/r/24475/diff/5/?file=659137#file659137line337>
> >
> > Why the change in sleep time? Why not "sleep 1000" which is used elsewhere for a "long" enough sleep.
The sleep time differs as we originally use a large image to test launching executor.
I agree we can just a consistent time now
> On Aug. 13, 2014, 8:36 p.m., Ian Downes wrote:
> > src/slave/containerizer/docker.cpp, line 852
> > <https://reviews.apache.org/r/24475/diff/5/?file=659133#file659133line852>
> >
> > Sorry if I missed it, but how does logging redirection get resumed when the slave restarts?
It's not handled yet, and Ben and I are working on that
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50485
-----------------------------------------------------------
On Aug. 13, 2014, 5:47 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 5:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50485
-----------------------------------------------------------
Ship it!
include/mesos/mesos.proto
<https://reviews.apache.org/r/24475/#comment88305>
s/an mesos/a Mesos/
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment88306>
s/a/an/
s/need/needs/
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment88307>
s/this but/but this/
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment88309>
s/to//
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment88344>
Sorry if I missed it, but how does logging redirection get resumed when the slave restarts?
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24475/#comment88345>
Why the change in sleep time? Why not "sleep 1000" which is used elsewhere for a "long" enough sleep.
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24475/#comment88346>
Ditto.
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24475/#comment88347>
Ditto.
src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24475/#comment88348>
Ditto. I don't follow why there are different sleep times in these tests?
- Ian Downes
On Aug. 12, 2014, 10:47 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2014, 10:47 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 13, 2014, 5:47 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs (updated)
-----
include/mesos/mesos.proto cc9f20e
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/flags.hpp 841de23
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50398
-----------------------------------------------------------
Rebased and added unit test for fetch
- Timothy Chen
On Aug. 13, 2014, 2:59 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 2:59 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/flags.hpp 841de23
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 13, 2014, 2:59 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs (updated)
-----
include/mesos/mesos.proto cc9f20e
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/flags.hpp 841de23
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 12, 2014, 8:45 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs (updated)
-----
include/mesos/mesos.proto cc9f20e
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/flags.hpp 841de23
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, line 720
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line720>
> >
> > Just re-iterating what I was commented on earlier, if you put the container in 'promises' then you could do the following here (and below in the other 'launch' too):
> >
> > fetch(...)
> > .onFailed([] () { promises.erase(containerId); })
> > .then(... _launch ...);
> >
> > Maybe I'm missing something else? If so, need more documentation please!
>
> Timothy Chen wrote:
> We can't use C++11 lambdas yet right? Thought we want to be backward compatible for now.
> But yes I was calling destroy just to clean up promises and not to create another callback method.
No, you can't use C++11 lambdas yet, I was just using them as pseudo code.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/slave.cpp, line 2397
> > <https://reviews.apache.org/r/24475/diff/2/?file=657112#file657112line2397>
> >
> > I thought the plan was to force CommandInfo even if ContainerInfo is present?
Yes, so that's why I reverted back to what slave was doing before, since really in the end we just want to make sure we either have commandInfo set or executorInfo set which is what slave and master are already checking for. ContainerInfo becomes an optional field to commandInfo as without ContainerInfo slave just follows the original flow.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, lines 246-247
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line246>
> >
> > It doesn't look like you're every putting containers into 'fetching'! Also, this is starting to get a lot more complicated, so I'd suggest adding both comments and CHECKs as much as possible (a CHECK would have caught your bug!). As in, IIUC, now you're expecting that 'fetching' and 'promises' are disjoint, and that a container "transitions" through from "fetching" to "launched", i.e., from the 'fetching' set to the 'promises' set. Am I correct? Let's make it easier for other people to understand these semantics.
> >
> > Also, I'm assuming that you didn't want to just put the container in 'promises' until after fetching is complete because it was easier to "cleanup" after fetching (whether it's successful or an failure) by just removing from 'fetching', but I wanted to mention that you can use .onFailed to cleanup 'promises' if fetching fails and otherwise just fall through to the next continuation where 'promises' still has the container.
I originally don't want to put in promises as I felt I don't want to let docker wait yet as it wasn't clear what I should put in the promise. But thinking about it as you said I'll just use promises and clean up in _fetch.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, lines 670-671
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line670>
> >
> > Please comment on MESOS-1686 since it was addressing this code that you're now killing.
>
> Timothy Chen wrote:
> I don't think MESOS-1686 is related at all?
Oops! I meant MESOS-1691.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, line 127
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line127>
> >
> > Can we use the continuation syntax to name this method? It'll make it easier for someone to do a broad sweep and add C++11 lambdas later.
I'm still going to rename it differently than the other _fetch, since it's really only called on fetch failed. (_fetchFailed)
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, line 720
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line720>
> >
> > Just re-iterating what I was commented on earlier, if you put the container in 'promises' then you could do the following here (and below in the other 'launch' too):
> >
> > fetch(...)
> > .onFailed([] () { promises.erase(containerId); })
> > .then(... _launch ...);
> >
> > Maybe I'm missing something else? If so, need more documentation please!
We can't use C++11 lambdas yet right? Thought we want to be backward compatible for now.
But yes I was calling destroy just to clean up promises and not to create another callback method.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, lines 670-671
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line670>
> >
> > Please comment on MESOS-1686 since it was addressing this code that you're now killing.
I don't think MESOS-1686 is related at all?
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/#review50135
-----------------------------------------------------------
The biggest thing this review could use is some tests which exercise the new fetching control flows in the DockerContainerizer. What happens when fetching fails?
Otherwise, looking pretty good!
include/mesos/mesos.proto
<https://reviews.apache.org/r/24475/#comment87719>
Please move this just below 'command' and also add a comment above both of them that explains the semantics. Same thing in ExecutorInfo too please!
include/mesos/mesos.proto
<https://reviews.apache.org/r/24475/#comment87720>
Why 100?
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment87721>
We'll likely want to make this a flag.
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment87722>
s/var/variable/ ;-)
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment87723>
Was there something more you wanted to say here? ;-)
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment87724>
s/runEnv/environment/
src/docker/docker.cpp
<https://reviews.apache.org/r/24475/#comment87725>
Can you add to the comment for posterity why you didn't just cd into the directory when you ran this command?
src/slave/containerizer/containerizer.hpp
<https://reviews.apache.org/r/24475/#comment87726>
Add newline please.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87727>
Can we use the continuation syntax to name this method? It'll make it easier for someone to do a broad sweep and add C++11 lambdas later.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87731>
It doesn't look like you're every putting containers into 'fetching'! Also, this is starting to get a lot more complicated, so I'd suggest adding both comments and CHECKs as much as possible (a CHECK would have caught your bug!). As in, IIUC, now you're expecting that 'fetching' and 'promises' are disjoint, and that a container "transitions" through from "fetching" to "launched", i.e., from the 'fetching' set to the 'promises' set. Am I correct? Let's make it easier for other people to understand these semantics.
Also, I'm assuming that you didn't want to just put the container in 'promises' until after fetching is complete because it was easier to "cleanup" after fetching (whether it's successful or an failure) by just removing from 'fetching', but I wanted to mention that you can use .onFailed to cleanup 'promises' if fetching fails and otherwise just fall through to the next continuation where 'promises' still has the container.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87733>
Proper capitalization please: 'URIs' over 'uris'.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87732>
Can you put the .then on the next line? It's easier to read and becoming more standard in the code base.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87734>
Please comment on MESOS-1686 since it was addressing this code that you're now killing.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24475/#comment87736>
Just re-iterating what I was commented on earlier, if you put the container in 'promises' then you could do the following here (and below in the other 'launch' too):
fetch(...)
.onFailed([] () { promises.erase(containerId); })
.then(... _launch ...);
Maybe I'm missing something else? If so, need more documentation please!
src/slave/slave.cpp
<https://reviews.apache.org/r/24475/#comment87739>
I thought the plan was to force CommandInfo even if ContainerInfo is present?
- Benjamin Hindman
On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2014, 7:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto cc9f20e
> src/docker/docker.hpp 98b2d60
> src/docker/docker.cpp 1cba381
> src/slave/containerizer/containerizer.hpp 02754cd
> src/slave/containerizer/containerizer.cpp c91ba38
> src/slave/containerizer/docker.cpp 904cdd3
> src/slave/containerizer/mesos/containerizer.cpp 694c9d1
> src/slave/slave.cpp 787bd05
> src/tests/docker_containerizer_tests.cpp a559836
> src/tests/docker_tests.cpp 4ef1df4
>
> Diff: https://reviews.apache.org/r/24475/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 10, 2014, 7:39 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs
-----
include/mesos/mesos.proto cc9f20e
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 10, 2014, 7:39 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Changes
-------
rebased on new changes.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs (updated)
-----
include/mesos/mesos.proto cc9f20e
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 24475: Add new Docker configurations
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24475/
-----------------------------------------------------------
(Updated Aug. 7, 2014, 10:01 p.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
Repository: mesos-git
Description
-------
Added new DockerInfo for future docker options, and allow command uris to be fetched and mapped into docker before launching docker container.
Diffs
-----
include/mesos/mesos.proto efb4239
src/docker/docker.hpp 98b2d60
src/docker/docker.cpp 1cba381
src/slave/containerizer/containerizer.hpp 02754cd
src/slave/containerizer/containerizer.cpp c91ba38
src/slave/containerizer/docker.cpp 904cdd3
src/slave/containerizer/mesos/containerizer.cpp 694c9d1
src/slave/slave.cpp 787bd05
src/tests/docker_containerizer_tests.cpp a559836
src/tests/docker_tests.cpp 4ef1df4
Diff: https://reviews.apache.org/r/24475/diff/
Testing
-------
make check
Thanks,
Timothy Chen