You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jojy Varghese <jo...@mesosphere.io> on 2015/11/17 00:15:16 UTC
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 16, 2015, 11:15 p.m.)
Review request for mesos and Timothy Chen.
Changes
-------
minor fixes.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
Diffs (updated)
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106778
-----------------------------------------------------------
src/cli/execute.cpp (line 421)
<https://reviews.apache.org/r/40371/#comment165550>
I have the checks here as it lets the user know the error right away as opposed to later.
- Jojy Varghese
On Nov. 16, 2015, 11:15 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2015, 11:15 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 16, 2015, 11:55 p.m., Timothy Chen wrote:
> > src/cli/execute.cpp, line 421
> > <https://reviews.apache.org/r/40371/diff/2/?file=1127646#file1127646line421>
> >
> > Instead of failing here and checking again in the Framework, how about just putting this check and wanring inside and leave it out here?
I have the checks here as it lets the user know the error right away as opposed to later.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106768
-----------------------------------------------------------
On Nov. 17, 2015, 2:52 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 2:52 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106768
-----------------------------------------------------------
src/cli/execute.cpp (line 116)
<https://reviews.apache.org/r/40371/#comment165535>
I think we should have a default (Mesos perhaps), and also I don't see why we should need to upper case this where every other flag is lower cased.
src/cli/execute.cpp (line 421)
<https://reviews.apache.org/r/40371/#comment165537>
Instead of failing here and checking again in the Framework, how about just putting this check and wanring inside and leave it out here?
- Timothy Chen
On Nov. 16, 2015, 11:15 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2015, 11:15 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106762
-----------------------------------------------------------
Bad patch!
Reviews applied: [40285]
Failed command: ./support/apply-review.sh -n -r 40285
Error:
2015-11-16 23:31:10 URL:https://reviews.apache.org/r/40285/diff/raw/ [2413/2413] -> "40285.patch" [1]
error: patch failed: src/slave/containerizer/mesos/provisioner/docker/puller.cpp:18
error: src/slave/containerizer/mesos/provisioner/docker/puller.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 16, 2015, 11:15 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2015, 11:15 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107151
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40371]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Nov. 19, 2015, 12:39 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2015, 12:39 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107446
-----------------------------------------------------------
Thanks Jojy, I'll fix these myself and merge.
- Timothy Chen
On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 7:45 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107371
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40371]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 7:45 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107445
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 7:45 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107444
-----------------------------------------------------------
src/cli/execute.cpp (line 237)
<https://reviews.apache.org/r/40371/#comment166554>
Probably unnecessary here.
src/cli/execute.cpp (line 418)
<https://reviews.apache.org/r/40371/#comment166555>
This is not true, we don't need a docker image for mesos containerizer strictly speaking.
- Timothy Chen
On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 7:45 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 20, 2015, 7:45 a.m.)
Review request for mesos and Timothy Chen.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=docker
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=mesos
Diffs
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107192
-----------------------------------------------------------
Bad patch!
Reviews applied: [40284, 40285]
Failed command: ./support/apply-review.sh -n -r 40285
Error:
2015-11-19 16:15:38 URL:https://reviews.apache.org/r/40285/diff/raw/ [3128/3128] -> "40285.patch" [1]
error: patch failed: src/slave/containerizer/mesos/provisioner/docker/puller.cpp:18
error: src/slave/containerizer/mesos/provisioner/docker/puller.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 19, 2015, 3:37 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2015, 3:37 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 19, 2015, 3:37 p.m.)
Review request for mesos and Timothy Chen.
Changes
-------
added back check for docker image in main.
Repository: mesos
Description (updated)
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=docker
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=mesos
Diffs (updated)
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 19, 2015, 12:39 a.m.)
Review request for mesos and Timothy Chen.
Changes
-------
review addressed.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
Diffs (updated)
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Guangya Liu <gy...@gmail.com>.
> On 十一月 17, 2015, 3:12 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, lines 216-228
> > <https://reviews.apache.org/r/40371/diff/3/?file=1127771#file1127771line216>
> >
> > Can you please explain more why using mesos containerizer can also run a docker container? Thanks!
>
> Jojy Varghese wrote:
> The '''Image::DOCKER''' in this context is image type and not container type. Image type could be '''docker''' or '''appc'''.
Thanks Jojy, but what is the difference of those two command?
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>>>>>>>>The above command will make the task running in docker containers.
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>>>>>>>>What does this one?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106789
-----------------------------------------------------------
On 十一月 19, 2015, 12:39 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated 十一月 19, 2015, 12:39 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Nov. 17, 2015, 3:12 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, lines 216-228
> > <https://reviews.apache.org/r/40371/diff/3/?file=1127771#file1127771line216>
> >
> > Can you please explain more why using mesos containerizer can also run a docker container? Thanks!
The '''Image::DOCKER''' in this context is image type and not container type. Image type could be '''docker''' or '''appc'''.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106789
-----------------------------------------------------------
On Nov. 17, 2015, 3:01 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 3:01 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106789
-----------------------------------------------------------
src/cli/execute.cpp (lines 216 - 228)
<https://reviews.apache.org/r/40371/#comment165562>
Can you please explain more why using mesos containerizer can also run a docker container? Thanks!
- Guangya Liu
On 十一月 17, 2015, 3:01 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated 十一月 17, 2015, 3:01 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106790
-----------------------------------------------------------
Bad patch!
Reviews applied: [40285]
Failed command: ./support/apply-review.sh -n -r 40285
Error:
2015-11-17 03:19:40 URL:https://reviews.apache.org/r/40285/diff/raw/ [2368/2368] -> "40285.patch" [1]
error: patch failed: src/slave/containerizer/mesos/provisioner/docker/puller.cpp:18
error: src/slave/containerizer/mesos/provisioner/docker/puller.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 17, 2015, 3:01 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 3:01 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review107104
-----------------------------------------------------------
src/cli/execute.cpp (line 237)
<https://reviews.apache.org/r/40371/#comment166033>
As suggested below, driver->abort() here and print the unsupported containerizer.
The advantage is that we only have one place to add or remove containerizers.
src/cli/execute.cpp (line 418)
<https://reviews.apache.org/r/40371/#comment166032>
IMO it's simpler to just have one place to check this that's happening in the CommandScheduler code, as you already have a check for mesos or docker.
I think it's not necessary to add checks in both places, because then it's not clear when someone else comes and add another containerizer support, that why they need to add here in line 417 but also in the resourceOffer callback.
How about just pass the flag in without any checks here, and in the CommandScheduler you can detect a unsupported containerizer and abort the driver.
- Timothy Chen
On Nov. 17, 2015, 3:01 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 3:01 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 17, 2015, 3:01 a.m.)
Review request for mesos and Timothy Chen.
Changes
-------
changed containerizer type to string.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
Diffs (updated)
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 17, 2015, 2:52 a.m.)
Review request for mesos and Timothy Chen.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
Diffs
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/#review106782
-----------------------------------------------------------
Bad patch!
Reviews applied: [40284, 40285]
Failed command: ./support/apply-review.sh -n -r 40285
Error:
2015-11-17 02:01:06 URL:https://reviews.apache.org/r/40285/diff/raw/ [2104/2104] -> "40285.patch" [1]
error: patch failed: src/slave/containerizer/mesos/provisioner/docker/puller.cpp:18
error: src/slave/containerizer/mesos/provisioner/docker/puller.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Nov. 17, 2015, 1:46 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 1:46 a.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a new option 'containerizer' to disambiguate the two containerizers.
>
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=DOCKER
>
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=MESOS
>
>
> Diffs
> -----
>
> src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
>
> Diff: https://reviews.apache.org/r/40371/diff/
>
>
> Testing
> -------
>
> tested the two containerizer locally with simple commands.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 40371: Changed mesos-execute to add containerizer
option.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40371/
-----------------------------------------------------------
(Updated Nov. 17, 2015, 1:46 a.m.)
Review request for mesos and Timothy Chen.
Changes
-------
review comments.
Repository: mesos
Description
-------
Since docker_image option could be used for mesos and docker containerizer,
introduced a new option 'containerizer' to disambiguate the two containerizers.
New usage:
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_docker --docker_image=ubuntu --containerizer=DOCKER
mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
--name=test_mesos --docker_image=ubuntu --containerizer=MESOS
Diffs (updated)
-----
src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc
Diff: https://reviews.apache.org/r/40371/diff/
Testing
-------
tested the two containerizer locally with simple commands.
Thanks,
Jojy Varghese