> On July 30, 2014, 8:12 p.m., Jay Buffington wrote:
> > src/slave/containerizer/docker.cpp, line 523
> > <https://reviews.apache.org/r/23771/diff/5/?file=645526#file645526line523>
> >
> >     This should be docker:// (two slashes, not three).  With a private 
> > registry I'll set image to 
> > docker://docker-registry.example.com/jaybuff/centos:6u5
> >     
> >     Same goes for line 537.
> 
> Timothy Chen wrote:
>     There is no standard of how a docker image should be prefixed 
> unfortunately, and this is how docker images are configured in Deimos as the 
> argument goes that it conforms more towards to the standards with three 
> slashes (https://github.com/mesosphere/deimos/issues/7)

My reading of that deimos issue is that burke said "it should be two slashes, 
not three" and then solidsnack agreed and made it two slashes with this commit: 
https://github.com/mesosphere/deimos/commit/7b9fd0d2d335edca75379880df5252f646d51994


> On July 30, 2014, 8:12 p.m., Jay Buffington wrote:
> > src/slave/containerizer/docker.cpp, line 525
> > <https://reviews.apache.org/r/23771/diff/5/?file=645526#file645526line525>
> >
> >     What does returning false here *mean*?  I think it is supposed to mean 
> > "the docker containerizer is passing on this task, the next containerizer 
> > in the list should make an attempt"
> >     
> >     But that's not what's happening here.  If the 
> >     scheduler doesn't prefix ContainerInfo's image with docker:/// "failed 
> > to start: TaskInfo/ExecutorInfo not supported" shows up in the log and the 
> > task fails to start. See slave/slave.cpp line 2443.
> 
> Timothy Chen wrote:
>     I think the place you see in slave.cpp is the output of the 
> ComposingContainerizer, which it internally traverses all the containerizers 
> that is configured and tries to launch each one. When it finally returns 
> false, it means it has tried all configured containerizers not just docker. 
> You can look at the containerizer/composing.cpp to see more details.

Okay, but the error message is still bogus, right?  TaskInfo/ExecutorInfo is 
supported with this patch now.  The error message should more accurately read 
"No containerizer can launch task <task_id>".

When a containerize passes on a task, it should log the reason why.  In my 
above example, it should log an info message such as "ContainerInfo image is 
not a docker image (it is not prefixed with docker:///)"

That will make debugging when writing a scheduler that uses the docker 
containerizer much easier.


- Jay


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


On July 30, 2014, 8:42 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23771/
> -----------------------------------------------------------
> 
> (Updated July 30, 2014, 8:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Docker implementation.
> This is all the docker code Ben, I and Yifan worked on excluding the 
> composing containerizer patches.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 45afcd1 
>   src/common/status_utils.hpp 1980551 
>   src/docker/docker.hpp PRE-CREATION 
>   src/docker/docker.cpp PRE-CREATION 
>   src/examples/docker_no_executor_framework.cpp PRE-CREATION 
>   src/health-check/main.cpp 707810a 
>   src/launcher/executor.cpp 9c80848 
>   src/linux/cgroups.hpp decad9d 
>   src/linux/cgroups.cpp 6a73dd7 
>   src/master/master.cpp 251b699 
>   src/slave/containerizer/containerizer.cpp 1b71f33 
>   src/slave/containerizer/docker.hpp PRE-CREATION 
>   src/slave/containerizer/docker.cpp PRE-CREATION 
>   src/slave/containerizer/external_containerizer.cpp 3f28d85 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 780037b 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 3265a80 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 8c476c7 
>   src/slave/containerizer/isolators/cgroups/mem.cpp e8d1e35 
>   src/slave/containerizer/isolators/posix.hpp 17bbd10 
>   src/slave/flags.hpp 1fe7b7d 
>   src/slave/slave.cpp f42ab60 
>   src/tests/cgroups_tests.cpp 01cf498 
>   src/tests/docker_containerizer_tests.cpp PRE-CREATION 
>   src/tests/docker_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 434b3f7 
>   src/tests/flags.hpp a003e7f 
>   src/tests/script.cpp 15a6542 
>   src/usage/usage.hpp 5a76746 
>   src/usage/usage.cpp 29014d1 
> 
> Diff: https://reviews.apache.org/r/23771/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Reply via email to