[go: up one dir, main page]

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

(daemon/graphdriver/zfs) Fixes #43080 #48520

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arcenik
Copy link
@arcenik arcenik commented Sep 17, 2024

Fixes #43080

- What I did

I updated the Remove function to ignore the "dataset does not exist" error

- How I did it

Doing a strings.Contains of "dataset does not exist" in err

- How to verify it

- Description for the changelog

Ignores "dataset does not exist" error when removing dataset on ZFS (#43080)

image

@arcenik arcenik force-pushed the 43080-zfs-destroy-missing-volume-fails branch from ffaaae8 to 2189e47 Compare September 17, 2024 18:53
@@ -361,6 +361,11 @@ func (d *Driver) Remove(id string) error {
name := d.zfsPath(id)
dataset := zfs.Dataset{Name: name}
err := dataset.Destroy(zfs.DestroyRecursive)
if err != nil && strings.Contains(fmt.Sprint(err), "dataset does not exist") {
Copy link

Choose a reason for hiding this comment

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

you may probably do a stricter check.

errZfs, ok := err.(zfs.Error)
if ok && errZfs.Stderr == "dataset does not exist" {
   ...
}

or go with err.Error() instead of fmt.Sprint(err), see https://pkg.go.dev/builtin@go1.23.1#error

Happy Go hacking mate!

Copy link
Author
@arcenik arcenik Sep 18, 2024

Choose a reason for hiding this comment

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

Done. Unfortunately, the stderr field contains the whole output of the zfs command

Stderr: "cannot open 'rpool/zzztest/doesnotexist': dataset does not exist\n",

errZfs, isZfsError := err.(*zfs.Error)
if isZfsError && strings.HasSuffix(errZfs.Stderr, "dataset does not exist\n") {
logger := log.G(context.TODO()).WithField("storage-driver", "zfs")
logger.Warnf("Tried to destroy inexistant dataset %s", name)
Copy link
Member
@AkihiroSuda AkihiroSuda Sep 18, 2024

Choose a reason for hiding this comment

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

Suggested change
logger.Warnf("Tried to destroy inexistant dataset %s", name)
logger.WithError(err).Warnf("Tried to destroy inexistant dataset %q", name)

Copy link
Member

Choose a reason for hiding this comment

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

Also please squash commits

Ignore "dataset does not exist" error in Remove function

Signed-off-by: François Scala <github@arcenik.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Driver "zfs" failed to remove root filesystem: "cannot open '<dataset-path>': dataset does not exist"
4 participants