Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove created volumes when taking down a compose stack #690

Closed
wants to merge 2 commits into from

Conversation

andyautida14
Copy link

What does this PR do?

Cleans up orphaned volumes when taking down a Docker Compose stack.

Why is it important?

Cleaning up after your tests is important.

Related issues

@andyautida14 andyautida14 requested a review from a team as a code owner December 16, 2022 11:02
Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Thanks for this! Could you please add a test for it? 🙏

@andyautida14
Copy link
Author

@mdelapenya just added the tests. Thanks!

@@ -157,6 +157,7 @@ func (d *dockerCompose) Down(ctx context.Context, opts ...StackDownOption) error
options := stackDownOptions{
DownOptions: api.DownOptions{
Project: d.project,
Volumes: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @andyautida14 this will always remove the volumes. Do you think we can make it optional, creating a RemoveVolumes functional option, exactly the same as in

// RemoveOrphans will cleanup containers that are not declared on the compose model but own the same labels
type RemoveOrphans bool
func (ro RemoveOrphans) applyToStackUp(o *stackUpOptions) {
o.RemoveOrphans = bool(ro)
}
func (ro RemoveOrphans) applyToStackDown(o *stackDownOptions) {
o.RemoveOrphans = bool(ro)
}
, only for the down options?

That way users would be able to configure if they need to clean up the volumes they create

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, sure. I'll try to find some time this week.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW @andyautida14 please take a look at #650, which was merged after your PR: it moves the compose code to a separate module

@mdelapenya
Copy link
Collaborator

@andyautida14 are you still interested in this PR? We'd like to include it, although there are missing pieces regarding the functional opts and the conflicts with current code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Remove created volumes on "down"
2 participants