Skip to content

Don't prune volumes on docker system prune#118

Merged
mlaventure merged 1 commit into
docker:masterfrom
cpuguy83:no_prune_volume
May 30, 2017
Merged

Don't prune volumes on docker system prune#118
mlaventure merged 1 commit into
docker:masterfrom
cpuguy83:no_prune_volume

Conversation

@cpuguy83
Copy link
Copy Markdown
Collaborator

@cpuguy83 cpuguy83 commented May 22, 2017

Volumes tend to carry important data and pruning them on docker system prune can easily cause unwanted data loss.

Let's play it safe and not prune volumes on system prune.

Throwing this out there.

@mlaventure
Copy link
Copy Markdown
Contributor

mlaventure commented May 22, 2017

Maybe make it an option? I mean system prune or any other destructive operation are not to be used if you don't know what you're doing 👼

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented May 23, 2017

Making it an option sounds good. docker-compose down has a -v option for removing volumes. The default is to only remove containers and networks.

@albers
Copy link
Copy Markdown
Collaborator

albers commented May 23, 2017

+1 for an option.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented May 30, 2017

Yes, I think having an option could make sense, on the other hand, calling docker volume prune to cleanup volumes is extra explicit, so may work as well.

Open to both options 😊

@cpuguy83
Copy link
Copy Markdown
Collaborator Author

cpuguy83 commented May 30, 2017

We can add an option. THe reason I didn't is it's kind of weird with the existing "--all" option (which is just weird anyway).

@cpuguy83
Don't prune volumes on docker system prune
37fd612 
Volumes tend to carry important data and pruning them on `docker system
prune` can easily cause unwanted data loss.

Let's play it safe and not prune volumes on `system prune` by default,
and instead provide an option.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Copy Markdown
Collaborator Author

cpuguy83 commented May 30, 2017

Added the option.

vdemeester
vdemeester approved these changes May 30, 2017
View reviewed changes
Copy link
Copy Markdown
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐯
the --all option is a little bit weird. Could we (in a follow-up PR) deprecated it, hide it and renaming it to --all-images or something ? 👼

mlaventure
mlaventure approved these changes May 30, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@mlaventure mlaventure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mlaventure mlaventure merged commit f3cb13c into docker:master May 30, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.06.0 milestone May 30, 2017
@cpuguy83 cpuguy83 deleted the no_prune_volume branch May 30, 2017 17:21
@dnephin dnephin mentioned this pull request May 30, 2017
Closed
This was referenced Jun 27, 2017
Merged
Merged
@thaJeztah thaJeztah modified the milestones: 17.07.0, 17.06.0 Jun 30, 2017
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 30, 2017

This didn't make it into 17.06, so updated the milestone

@albers albers mentioned this pull request Jun 30, 2017
Merged
@andrewhsu andrewhsu mentioned this pull request Jul 11, 2017
Merged
@thaJeztah thaJeztah modified the milestones: 17.06.1, 17.07.0 Jul 21, 2017
trapier pushed a commit to trapier/cli that referenced this pull request Sep 30, 2019
@andrewhsu
Merge pull request docker#118 from GordonTheTurtle/sync-with-upstream…
f74e914 
…-master-f40f9c240ab03d20ae16ba97ec97ecb2cad50558

[master] sync to upstream master f40f9c2
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
@andrewhsu
Merge pull request docker#118 from andrewhsu/rc1
c257617 
bump version to 17.06.1-ce-rc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdemeester vdemeester vdemeester approved these changes

@dnephin dnephin Awaiting requested review from dnephin

@thaJeztah thaJeztah Awaiting requested review from thaJeztah

+1 more reviewer

@mlaventure mlaventure mlaventure approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

impact/changelog process/cherry-picked status/1-design-review

Projects

None yet

Milestone

17.06.1

Development

Successfully merging this pull request may close these issues.

7 participants

@cpuguy83 @mlaventure @dnephin @albers @thaJeztah @vdemeester @GordonTheTurtle