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

Added fixes for issue #971(Mounted volume can be deleted) #974

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

click2cloud-lamda
Copy link
Contributor

Fixed issue #971

In this PR, if the volume is mounted, it doesn't allow the volume to be removed.

D:\GolandProjects\github.com\nerdctl\cmd\nerdctl>nerdctl run -d -v testVolume:C:\src mcr.microsoft.com/windows/nanoserver:20H2
23043c648edcfd53309d2d365676072bb5b6e24cc06e4a5c61769860a1015edf

D:\GolandProjects\github.com\nerdctl\cmd\nerdctl>nerdctl volume rm testVolume
time="2022-04-11T16:45:33+05:30" level=error msg="Remove Volume: \"testVolume\" failed" error="Volume \"testVolume\" is in use"

@AkihiroSuda
Copy link
Member

Could you delete the commit 53bc54e that seems to be duplicated from #924?

Also, next time please make the PR title more descriptive.

@AkihiroSuda AkihiroSuda changed the title Added fixes for issue #971 Added fixes for issue #971(Mounted volume can be deleted) Apr 12, 2022
if found = checkVolume(&mount.Mounts, volGet.Mountpoint); found {
found = true
var verr error
verr = fmt.Errorf("%s %q %s", "Volume", name, "is in use")
Copy link
Member

Choose a reason for hiding this comment

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

No need to use %s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay

return err
}
defer cancel()
containers, _ := client.Containers(ctx)
Copy link
Member

Choose a reason for hiding this comment

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

catch err

if removednames != nil {
rNames, err := volStore.Remove(removednames)
if err != nil {
panic(err)
Copy link
Member

Choose a reason for hiding this comment

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

why panic

if removednames != nil {
rNames, err := volStore.Remove(removednames)
if err != nil {
panic(err)
Copy link
Member

Choose a reason for hiding this comment

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

I think just return error directly would be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill change it.

return err
}
var mount *specs.Spec
var removednames []string
Copy link
Member

Choose a reason for hiding this comment

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

var removedNames would be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used it for volumes that have to be deleted.

Signed-off-by: Raymond Mathew <raymond.mathew@click2cloud.net>
@AkihiroSuda AkihiroSuda added this to the v0.19.0 (tentative) milestone Apr 15, 2022
@AkihiroSuda AkihiroSuda merged commit f92bae5 into containerd:master Apr 15, 2022
@AkihiroSuda AkihiroSuda linked an issue Apr 19, 2022 that may be closed by this pull request
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.

Mounted volume can be deleted
3 participants