mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Greg Mann" <g...@mesosphere.io>
Subject Re: Review Request 38910: Added `-v` flag to `docker rm`.
Date Wed, 07 Oct 2015 00:27:05 GMT


> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > <https://reviews.apache.org/r/38910/diff/1/?file=1088143#file1088143line681>
> >
> >     wondering this behavior should be defaulted or not. We might be overloading
stop with more than what it should be doing isnt it? Do we always want to force remove the
volumes when docker stops?
> 
> Greg Mann wrote:
>     My & Tim's original thought was that because persistent volumes aren't explicitly
implemented for the Docker containerizer currently, we could safely make `-v` the default
behavior. However, after considering a bit more, I'm thinking that existing user workflows
might establish a Docker volume on an agent with the intention of returning to it later with
a subsequent task, in which case defaulting to `-v` would break this workflow, so making this
an optional argument may be a good idea. I'll have to think about what is the best way to
pass this option to the agent, any thoughts on that matter would be appreciated!

After further review, I do think that we should make the default behavior include the `-v`
flag. By default, Docker will create directories in `/var/lib/docker/aufs/`, `/var/lib/docker/vfs`,
etc. depending on the filesystem used. These directories contain information related to the
Docker image itself, and they will not be deleted if the `-v` flag isn't used. Thus, if an
agent runs long enough and runs enough new Docker images, it can fill up the disk with these
default volumes.

While it's possible that a user might hack together a "persistent volume" using Docker volumes
as they're currently implemented, we should discourage this in favor of utilizing Mesos-monitored
persistent volumes. Always using `docker rm -v` allows us to properly clean the agent, leaving
it in a good state after the containerized task has completed.

It's also worth noting that `docker rm -v` will NOT remove volumes where a host path has been
explicitly mounted into the container (i.e. using `docker run -v /host/path:/container/path
mycontainer`). So, in fact, some volumes will be unaffected by this change. The primary purpose
of this patch is to clean the agent by removing the default volumes created in `/var/lib/docker/`.


- Greg


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


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
>     https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 VM, but
this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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