You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/12/27 01:22:43 UTC
Review Request 41731: Removed docker puller flag.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
Repository: mesos
Description
-------
Removed docker puller flag.
Diffs
-----
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
Diff: https://reviews.apache.org/r/41731/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 41731: Removed docker puller flag.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review112336
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp (line 72)
<https://reviews.apache.org/r/41731/#comment172808>
Do you still need flags here?
We typically try to be more explict and only pass in necessary parameters. All the flags validation should be done at the '::create()' method.
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp (line 73)
<https://reviews.apache.org/r/41731/#comment172813>
s/imagesDir/archivesDir/
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp (lines 79 - 82)
<https://reviews.apache.org/r/41731/#comment172811>
Can you introduce a LocalPuller::create and put this logic there?
src/slave/containerizer/mesos/provisioner/docker/puller.cpp (line 53)
<https://reviews.apache.org/r/41731/#comment172816>
No need for this temp variable?
src/slave/flags.hpp (line 55)
<https://reviews.apache.org/r/41731/#comment172798>
Hum, this flag is confusing to me. Sounds like the docker images (e.g., busybox,ubuntu...) used by Mesos.
Can we keep the name 'docker_registry'?
src/slave/flags.cpp (lines 134 - 135)
<https://reviews.apache.org/r/41731/#comment172804>
Can you briefly explain what will be there if a file location is specified:
```
or a local path (i.e., file://xxx) in which Docker image archives (result of 'docker save') are stored.
```
src/tests/containerizer/provisioner_docker_tests.cpp (line 1462)
<https://reviews.apache.org/r/41731/#comment172817>
s/imageDir/archivesDir/
Can you fix all the occurances?
- Jie Yu
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Gilbert Song <so...@gmail.com>.
> On Dec. 26, 2015, 9:42 p.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line145>
> >
> > Could we use pure directory only for local puller?
> >
> > Instead of adding `file://` to the flag & strings::remove it, because some users may easily neglect a slash for this pattern.
>
> Timothy Chen wrote:
> We need to differentiate based on the flag's value to be local or registry as we consolidated a bunch of flags down to a single one now.
Is it possible to select local or registry by:
`if (!strings::startsWith(imagesUrl, "https://"))`
Not 100% sure it is safe enough. If no, we could drop this issue. However, we may still consider user with `--docker_image=file:///tmp/mesos/images/docker` VS `--docker_image=/tmp/mesos/images/docker` in the future.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
-----------------------------------------------------------
On Dec. 26, 2015, 4:22 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2015, 4:22 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 27, 2015, 5:42 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line145>
> >
> > Could we use pure directory only for local puller?
> >
> > Instead of adding `file://` to the flag & strings::remove it, because some users may easily neglect a slash for this pattern.
>
> Timothy Chen wrote:
> We need to differentiate based on the flag's value to be local or registry as we consolidated a bunch of flags down to a single one now.
>
> Gilbert Song wrote:
> Is it possible to select local or registry by:
>
> `if (!strings::startsWith(imagesUrl, "https://"))`
>
> Not 100% sure it is safe enough. If no, we could drop this issue. However, we may still consider user with `--docker_image=file:///tmp/mesos/images/docker` VS `--docker_image=/tmp/mesos/images/docker` in the future.
That's just not possible if we stick with this configuration.
Btw Jie I just realize that the downside of consolidating flags like this is that, users that want to use the docker registry will then have to specify the docker registry URL themselves everytime now, which we used to have a default value for a seperate flag. To me that's a pretty bad experience as well. What did you guys conclude last time when you guys discussed about this?
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
-----------------------------------------------------------
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 27, 2015, 5:42 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line145>
> >
> > Could we use pure directory only for local puller?
> >
> > Instead of adding `file://` to the flag & strings::remove it, because some users may easily neglect a slash for this pattern.
We need to differentiate based on the flag's value to be local or registry as we consolidated a bunch of flags down to a single one now.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
-----------------------------------------------------------
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111921
-----------------------------------------------------------
src/slave/flags.cpp (lines 135 - 136)
<https://reviews.apache.org/r/41731/#comment172213>
Could we use pure directory only for local puller?
Instead of adding `file://` to the flag & strings::remove it, because some users may easily neglect a slash for this pattern.
- Gilbert Song
On Dec. 26, 2015, 4:22 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 26, 2015, 4:22 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111912
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41715, 41727, 41728, 41731]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 27, 2015, 2:38 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp, line 79
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176470#file1176470line79>
> >
> > You might have to validate the flag for its format and maybe valid path.
Actually we assume it's file:// at this point, I can even add a CHECK() here.
We will know about valid path right when we do a look up in that folder.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111916
-----------------------------------------------------------
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111916
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp (line 79)
<https://reviews.apache.org/r/41731/#comment172203>
You might have to validate the flag for its format and maybe valid path.
- Jojy Varghese
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review112367
-----------------------------------------------------------
Ship it!
src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp (line 44)
<https://reviews.apache.org/r/41731/#comment172835>
I would still pass in const Flags& flags here like we did in RegistryPuller.
src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp (line 53)
<https://reviews.apache.org/r/41731/#comment172838>
```
explicit LocalPuller(Owned<LocalPullerProcess> _process);
```
const ref on Owned pointer does not make too much sense (since you need to transfer ownership).
src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp
<https://reviews.apache.org/r/41731/#comment172836>
Why copyable?
src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp (line 57)
<https://reviews.apache.org/r/41731/#comment172837>
Can you remove const here just to be consistent. const here does not enforce anything indeed.
- Jie Yu
On Dec. 30, 2015, 11:08 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2015, 11:08 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> docs/configuration.md a33e802a3ff1246d25f52b15da7905c5b22e339d
> docs/mesos-provisioner.md fdb298c2a954e903317ef56abbcfe2470a2dfd23
> src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 025d96c80529024f6a1c1e2c15e1eda513c680fd
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/flags.hpp 2b2679c1ae68d120756eaf81e5728d20791d6746
> src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review112420
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Dec. 30, 2015, 3:08 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2015, 3:08 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> docs/configuration.md a33e802a3ff1246d25f52b15da7905c5b22e339d
> docs/mesos-provisioner.md fdb298c2a954e903317ef56abbcfe2470a2dfd23
> src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 025d96c80529024f6a1c1e2c15e1eda513c680fd
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/flags.hpp 2b2679c1ae68d120756eaf81e5728d20791d6746
> src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/
-----------------------------------------------------------
(Updated Dec. 30, 2015, 11:08 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
Repository: mesos
Description
-------
Removed docker puller flag.
Diffs (updated)
-----
docs/configuration.md a33e802a3ff1246d25f52b15da7905c5b22e339d
docs/mesos-provisioner.md fdb298c2a954e903317ef56abbcfe2470a2dfd23
src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 025d96c80529024f6a1c1e2c15e1eda513c680fd
src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
src/slave/flags.hpp 2b2679c1ae68d120756eaf81e5728d20791d6746
src/slave/flags.cpp a60d3c8022aba93fbd17a46dfff601fb1b25bbee
src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
Diff: https://reviews.apache.org/r/41731/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 41731: Removed docker puller flag.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 146
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line146>
> >
> > Do I understand correctly that we change the default from "Pull from docker registry" to "pull from a local file location"? Wouldn't that impact users experimenting with Mesos? If this is not the case feel to drop.
>
> Timothy Chen wrote:
> The default has always been pulling from a local file location (docker_puller = local)
THX!
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111924
-----------------------------------------------------------
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 146
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line146>
> >
> > Do I understand correctly that we change the default from "Pull from docker registry" to "pull from a local file location"? Wouldn't that impact users experimenting with Mesos? If this is not the case feel to drop.
The default has always been pulling from a local file location (docker_puller = local)
> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 126
> > <https://reviews.apache.org/r/41731/diff/1/?file=1176474#file1176474line126>
> >
> > When changing these flags we should also update the documentation: http://mesos.apache.org/documentation/latest/configuration/
Ah forgot about it, I'll add it.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111924
-----------------------------------------------------------
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 41731: Removed docker puller flag.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41731/#review111924
-----------------------------------------------------------
src/slave/flags.cpp
<https://reviews.apache.org/r/41731/#comment172217>
When changing these flags we should also update the documentation: http://mesos.apache.org/documentation/latest/configuration/
src/slave/flags.cpp (line 136)
<https://reviews.apache.org/r/41731/#comment172216>
Do I understand correctly that we change the default from "Pull from docker registry" to "pull from a local file location"? Wouldn't that impact users experimenting with Mesos? If this is not the case feel to drop.
- Joerg Schad
On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2015, 12:22 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Removed docker puller flag.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp f71b572a32443a6715acb4d3541aec60e0437b30
> src/slave/containerizer/mesos/provisioner/docker/puller.cpp dd17acf6a6e5c306029198dbb2a7e2d059f87f75
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 7cdb15b9529eab82867b3470a016bb8ad09ff250
> src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9
> src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806
> src/tests/containerizer/provisioner_docker_tests.cpp bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98
>
> Diff: https://reviews.apache.org/r/41731/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>