mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.
Date Fri, 22 Jan 2016 23:09:11 GMT


> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > <https://reviews.apache.org/r/41892/diff/1/?file=1181052#file1181052line410>
> >
> >     (Sorry I just got time to come back to this).
> >     
> >     I don't exactly understand your suggestion about "add additional containerInfo.volumes()
in DockerContainerizerProcess::Container::create()", because the latter function actually
does not pass a `ContainerInfo`.
> >     
> >     Because the volume is actually included in `TaskInfo::ContainerInfo::volumes`,
we would need to mutate the `TaskInfo` before passing it to the `DockerExecutorProcess::launchTask()`
function, which I don't really know how to do it, and I even doubt whether it's a good idea
for logging/clarity purpose.
> >     
> >     Please let me know if I didn't understand your suggestion, or if you think we
should explore the other alternative (passing `hostPath` earlier in resource offer).

Sorry for being late on this.

My sugguestion is that we add some logics in DockerContainerProcess::Container::create(),
the 'create' function has both TaskINfo and ExecutorInfo. So, you should be able to get the
ContainerInfo from them. When you generate DockerContainerizerProcess::Container, you should
be able to generate a new ContainerInfo that has persistent volumes in it. Let me know if
you still don't understand.

FYI, docker just merged its mount propagation support for volumes. That means we can have
full persistent volume support. We need to mount the sandbox as a shared mount into the docker
container. ANy mounts under the sandbox mount will be automatically propergated to the docker
container.


- Jie


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
>     https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes from resources
passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard code slave's
work_dir. I also checked that the inferred directory actually exists to avoid framework messing
up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start the container,
which seems to belong to slave.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp cb58b7183c36d96b9ac4803c63980c278a50c97b

>   src/tests/containerizer/docker_tests.cpp 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> -------
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message