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

e2e: corrupted mount not attempting to recover. #4633

Open
iPraveenParihar opened this issue May 23, 2024 · 8 comments
Open

e2e: corrupted mount not attempting to recover. #4633

iPraveenParihar opened this issue May 23, 2024 · 8 comments
Labels
keepalive This label can be used to disable stale bot activiity in the repo

Comments

@iPraveenParihar
Copy link
Contributor

          > > replacing, `k8s.io/mount-utils => k8s.io/mount-utils v0.29.3`

this works!, So, will add a TODO: update once fixed in mount-utils and visit later? @Madhu-1

@iPraveenParihar Thank you, lets get it merged and we will revisit mount-utils, lets check what is changed in mount-utils release notes.

Originally posted by @Madhu-1 in #4614 (comment)

@iPraveenParihar
Copy link
Contributor Author

There is a change in the IsLikelyNotMountPoint method: kubernetes/mount-utils@v0.29.3...v0.30.2

From the method description, it is clear that IsLikelyNotMountPoint is not suitable for detecting bind mounts: https://github.com/kubernetes/mount-utils/blob/dfd453c575c09ba21aa9f45bf9a9ddb8724c5e0b/mount_linux.go#L498-L513

ceph-csi uses this method for fuse mount recovery to determine if the path is a valid MountPoint or not.

// IsMountPoint checks if the given path is mountpoint or not.
func IsMountPoint(mounter mount.Interface, p string) (bool, error) {
notMnt, err := mounter.IsLikelyNotMountPoint(p)
if err != nil {
return false, err
}
return !notMnt, nil
}

we need alternative method to be used?
thoughts @nixpanic @Madhu-1 ?

@nixpanic
Copy link
Member

Probably you can delete this function and call x.mounter.IsMountPoint(...) directly? I don't see why the indirection is beneficial for anything.

@iPraveenParihar
Copy link
Contributor Author

Probably you can delete this function and call x.mounter.IsMountPoint(...) directly? I don't see why the indirection is beneficial for anything.

IsMountPoint(...) is an expensive operation as it enumerates mount points on the Node and then does the evaluate operation on each mount point.
https://github.com/kubernetes/mount-utils/blob/dfd453c575c09ba21aa9f45bf9a9ddb8724c5e0b/mount.go#L72-L79

may be because of that, they chose IsLikelyNotMountPoint()?

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jun 19, 2024

Probably you can delete this function and call x.mounter.IsMountPoint(...) directly? I don't see why the indirection is beneficial for anything.

IsMountPoint(...) is an expensive operation as it enumerates mount points on the Node and then does the evaluate operation on each mount point. https://github.com/kubernetes/mount-utils/blob/dfd453c575c09ba21aa9f45bf9a9ddb8724c5e0b/mount.go#L72-L79

may be because of that, they chose IsLikelyNotMountPoint()?

@iPraveenParihar IsMountPoint is already used for RBD and IsLikelyNotMountPoint currently used only for cephfs. we can switch from IsLikelyNotMountPoint to IsMountPoint for cephfs as well.

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jun 19, 2024

@nixpanic #3281 in this PR we switch to IsLikelyNotMountPoint for specific case for rbd, if we switch back to IsMountPoint wont we get into the same problem again, can you please check or we need to still use IsLikelyNotMountPoint for specific cases and switch to IsMointPoint in util function.

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the wontfix This will not be worked on label Jul 19, 2024
@iPraveenParihar iPraveenParihar removed the wontfix This will not be worked on label Jul 23, 2024
@LinskId
Copy link

LinskId commented Aug 13, 2024

sorry, I might have missed it in your merged pr, but if indeed we identify a bind mount how do we recover from it?

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the wontfix This will not be worked on label Sep 13, 2024
@iPraveenParihar iPraveenParihar added keepalive This label can be used to disable stale bot activiity in the repo and removed wontfix This will not be worked on labels Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive This label can be used to disable stale bot activiity in the repo
Projects
None yet
Development

No branches or pull requests

4 participants