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

do not ignore "volume in use" errors when force-delete #31450

Merged
merged 1 commit into from Mar 6, 2017

Conversation

Projects
None yet
5 participants
@thaJeztah
Member

thaJeztah commented Mar 1, 2017

fixes #31446

When using docker volume rm -f, all errors were ignored, and volumes where Purged, even if they were still in use by a container. As a result, repeated calls to docker volume rm -f actually removed the volume.

The -f option was implemented to ignore errors in case a volume was already removed out-of-band by a volume driver plugin.

This patch changes the remove function to not ignore "volume in use" errors if -f is used. Other errors are still ignored as before.

- How to verify it

$ docker volume create my-volume
my-volume

$ docker create -v my-volume:/volume busybox

# then run `docker volume rm -f` multiple times; it should produce an error each time
$ docker volume rm my-volume
$ docker volume rm my-volume

# the volume should still be present
$ docker volume ls

- Description for the changelog

Fix a bug where docker volume rm -f was removing volumes that were still in use by a container.

- A picture of a cute animal (not mandatory but encouraged)

loving-mother-cow-and-calf1

@vdemeester

LGTM 🐸

Show outdated Hide outdated integration-cli/docker_cli_volume_test.go Outdated
Show outdated Hide outdated daemon/delete.go Outdated
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 1, 2017

Member

updated. ping @cpuguy83 PTAL

Member

thaJeztah commented Mar 1, 2017

updated. ping @cpuguy83 PTAL

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Mar 1, 2017

Contributor
15:17:33  - integration-cli/docker_cli_volume_test.go
15:17:33 
15:17:33 Please reformat the above files using "gofmt -s -w" and commit the result.
Contributor

cpuguy83 commented Mar 1, 2017

15:17:33  - integration-cli/docker_cli_volume_test.go
15:17:33 
15:17:33 Please reformat the above files using "gofmt -s -w" and commit the result.
do not ignore "volume in use" errors when force-delete
When using `docker volume rm -f`, all errors were ignored,
and volumes where Purged, even if they were still in
use by a container.

As a result, repeated calls to `docker volume rm -f`
actually removed the volume.

The `-f` option was implemented to ignore errors
in case a volume was already removed out-of-band
by a volume driver plugin.

This patch changes the remove function to not
ignore "volume in use" errors if `-f` is used.
Other errors are still ignored as before.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 1, 2017

Member

arg. looks like gogland doesn't format correctly, let me check my preferences again, and otherwise open a bugreport there.

anyway, fixed 😊

Member

thaJeztah commented Mar 1, 2017

arg. looks like gogland doesn't format correctly, let me check my preferences again, and otherwise open a bugreport there.

anyway, fixed 😊

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 2, 2017

Member

Test passed on Windows

17:02:49 PASS: docker_cli_volume_test.go:436: DockerSuite.TestVolumeCLIRmForceInUse	0.429s

Failure looks unrelated (flaky);

16:36:19 ----------------------------------------------------------------------
16:36:19 FAIL: docker_api_logs_test.go:15: DockerSuite.TestLogsAPIWithStdout
16:36:19 
16:36:19 docker_api_logs_test.go:50:
16:36:19     c.Fatal("timeout waiting for logs to exit")
16:36:19 ... Error: timeout waiting for logs to exit
16:36:19 
Member

thaJeztah commented Mar 2, 2017

Test passed on Windows

17:02:49 PASS: docker_cli_volume_test.go:436: DockerSuite.TestVolumeCLIRmForceInUse	0.429s

Failure looks unrelated (flaky);

16:36:19 ----------------------------------------------------------------------
16:36:19 FAIL: docker_api_logs_test.go:15: DockerSuite.TestLogsAPIWithStdout
16:36:19 
16:36:19 docker_api_logs_test.go:50:
16:36:19     c.Fatal("timeout waiting for logs to exit")
16:36:19 ... Error: timeout waiting for logs to exit
16:36:19 
@cpuguy83

LGTM

@vdemeester vdemeester merged commit 95cb748 into moby:master Mar 6, 2017

3 of 4 checks passed

windowsRS1 Jenkins build Docker-PRs-WoW-RS1 10884 has failed
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 31205 has succeeded
Details
janky Jenkins build Docker-PRs 39821 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.04.0 milestone Mar 6, 2017

@thaJeztah thaJeztah deleted the thaJeztah:fix-volume-force-remove branch Mar 6, 2017

@thaJeztah thaJeztah modified the milestones: 17.03.1, 17.04.0 Mar 6, 2017

@thaJeztah thaJeztah removed this from PRs in 17.03.2-maybe Mar 22, 2017

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 22, 2017

Member

cherry-picked into 17.03.1 through #31754

Member

thaJeztah commented Mar 22, 2017

cherry-picked into 17.03.1 through #31754

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment