Better pruning of docker volumes during make up/down

[KevinMind]

Relates to: mozilla/addons#15119
Relates to: mozilla/addons#15046

Description

Remove many totally unnecessary docker volumes and simplifies the management of others.

Context

There is a recently squashed upstream bug in docker where (supposed to be) anonymous volumes are not labelled correctly, causing our various docker commands to leave behind anonymous looking volumes. Over time these volumes pile up and result in out of storage errors and general slowness of docker.

This PR originally tried to address that issue but instead simply addresses the volume mount concurrency problem and reduces the surface area of the space problem by reducing the number of volumes our project depends on.

Testing

Make UP

Create an anonyomous dangling volumne

docker volume create

Note the hash name of the volume

Now run make up to ensure your containers and non dangling volumes are running.

make docker_clean_volumes

Expect that the volume you created is not destroyed, neither are any of the running container volumes

make down

This should remove anonymous volumes that were created when running make up, but should not remove the volume you created initially.

Bonus: Rerunning make docker_clean_volumes after running make down should either do nothing (if all volumes were cleaned up already) or remove any project associated anonymous volumes. Again, you original volume from the top should still be there.

Checklist

• Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
• Successfully verified the change locally.
• The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
• Add before and after screenshots (Only for changes that impact the UI).
• Add or update relevant docs reflecting the changes made.

[eviljeff]

Needs an issue 😉 - though I'm not sure I really understand the use-case, and why we need this clean-up script in our makefile - it seems more a general docker clean-up, and something that doesn't affect anything (other than a bit of lost disk space). Maybe @diox has other thoughts.

[KevinMind]

Needs an issue 😉 - though I'm not sure I really understand the use-case, and why we need this clean-up script in our makefile - it seems more a general docker clean-up, and something that doesn't affect anything (other than a bit of lost disk space). Maybe @diox has other thoughts.

It's not a bit... it can be dozens or over a hundred gigabytes of data. Essentially docker is not smart enough to clean up some of our orphan volumes and some of those contain huge data (our whole repository or memory cache from service X)

I regularly run into out of disk space errrors so finally decided to fix it.

The clean up script has been there for months, it just didn't work. Now it works.

[eviljeff]

I regularly run into out of disk space errrors so finally decided to fix it.

I have 600GB free and haven't done anything special to clean extra docker cruft - was this because you were working on docker optimizations (now complete?) or it's some symptom of a setup specific problem you have?

The clean up script has been there for months, it just didn't work. Now it works.

what script? The code it replaces is just docker volume prune --force

[diox]

Definitely needs an issue if only for tracking purposes. I've ran into this too, but I'm not sure such an agressive way of handling this is warranted in our own Makefile, as it would apply to all docker volumes on the host - including some the person running make up/down might want to keep, we don't know!

[KevinMind]

Definitely needs an issue if only for tracking purposes. I've ran into this too, but I'm not sure such an agressive way of handling this is warranted in our own Makefile, as it would apply to all docker volumes on the host - including some the person running make up/down might want to keep, we don't know!

@diox to your point..

True it removes all dangling volumes, but only filters for "dangling" volumes which means ones not associated with any container and not tagged by a user.. These volumes cannot serve any purpose as no container or compose project would be able to use them since they are already dangling/orphaned.

Additionally, we already had this as the intended outcome, it just hasn't worked.. maybe ever. But it was always there. So I'm confused why we are discussing the intent of the patch, when it's already there.

[KevinMind]

@eviljeff linked issue.

[eviljeff]

True it removes all dangling volumes, but only filters for "dangling" volumes which means ones not associated with any container and not tagged by a user.. These volumes cannot serve any purpose as no container or compose project would be able to use them since they are already dangling/orphaned.

The script special cases the mysql volume so isn't it possible another docker based project could also have special volumes that shouldn't be removed?

Additionally, we already had this as the intended outcome, it just hasn't worked.. maybe ever. But it was always there.

A command was there - docker volume prune --force. Now it's a more complicated and aggressive script. And it's worked well enough for me - I don't have a disk space problem. How about this be an optional Make recipe you can run every now and again?

[KevinMind]

Funny enough, this is actually a bug in docker compose moby/moby#48754 that seems to have just been fixed, like several days ago.

[KevinMind]

@eviljeff addressing some of your points:

1. current master will remove dangling volumes not associated with our docker compose project so the issue you described exists currently and is not introduce by this patch. In fact, I added a filter for our project to the clean command so now it should actually NOT remove volumes not associated with our project.
2. I think in general "it worked well enough for me" is not a very good reason to not work on an issue. That's equivalent to "it works on my machine" for QA. I've shipped many patches that didn't affect my particular dev environment because it impacted others. Just food for thought on how you might be thinking through the value prop here.
3. I found in docker (linked above) the "root" cause of not cleaning the anonymous volumes is that they are improperly labelled. docker apparently relies on a specific label to know which volumes are anonymous and docker compose is not adding those currently, so when figuring out which volumes to remove, it just skips them.. I've left a fix for that out for now and we can hope the upstream fix is shipped soon.

[eviljeff]

@eviljeff addressing some of your points:

1. current master will remove dangling volumes not associated with our docker compose project so the issue you described exists currently and is not introduce by this patch. In fact, I added a filter for our project to the clean command so now it should actually NOT remove volumes not associated with our project.

2. I think in general "it worked well enough for me" is not a very good reason to _not_ work on an issue. That's equivalent to "it works on my machine" for QA. I've shipped many patches that didn't affect my particular dev environment because it impacted others. Just food for thought on how you might be thinking through the value prop here.

3. I found in docker (linked above) the "root" cause of not cleaning the anonymous volumes is that they are improperly labelled. docker apparently relies on a specific label to know which volumes are anonymous and docker compose is not adding those currently, so when figuring out which volumes to remove, it just skips them.. I've left a fix for that out for now and we can hope the upstream fix is shipped soon.

I wasn't arguing that we shouldn't fix an issue I'm not experiencing, I was arguing we shouldn't make the user experience worse by automatically clearing stuff from Docker we didn't ourselves create, for an issue that had unclear impact (i.e. if it only affected you, or a small minority of users). If the script is limited to our created content, so it doesn't regress or otherwise introduce any harmful side effects, then no problem.

[KevinMind]

we shouldn't make the user experience worse by automatically clearing stuff from Docker we didn't ourselves create

Right, I'm only trying to point out that

we shouldn't make the user experience worse by automatically clearing stuff from Docker we didn't ourselves create

We are doing this, right now, on master. This patch doesn't introduce that problem, and now actually solves that problem.

[eviljeff]