Skip to content

Conversation

@hensur
Copy link
Contributor

@hensur hensur commented Aug 1, 2024

Currently waiting for openebs/lib-csi#21 to be merged. I'm replacing the upstream dependency with my fork, after lib-csi has been updated, I will remove that replace.

Pull Request template

Please, go through these steps before you submit a PR.

Why is this PR required? What issue does it fix?:
On Nodes with high mount activity, older versions of k8s mount-utils (or utils/mount) would call ConsistentRead to workaround a kernel bug which would skip mount entries on mount activity during a read.
See kubernetes/mount-utils@4ae857e

We noticed that the zfs provisioner would return an InconsistentRead error when the node was attaching a lot of volumes (mostly after reboots).

What this PR does?:

replaces the k8s.io/utils/mount dependency with k8s.io/mount-utils

the upstream package has been renamed: https://github.com/kubernetes/utils/tree/master/mount

The new version also includes the aforementioned fix.

Does this PR require any upgrade changes?:
No

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
I'm not exactly sure why go mod tidy deems it necessary to also bump the module version (as well as some package versions). I assume some library requires an up to date go version, which is why this module now has to require that version as well.

Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/website is used to track them:

@Abhinandan-Purkait
Copy link
Member

I see the vendor directory being checked in. I don't think we should do that. cc @niladrih

@niladrih
Copy link
Member

niladrih commented Aug 7, 2024

@hensur We're not using a vendor directory. Moreover, if you rebase you branch with develop, you'd see that go version used is now 1.20 and I think mount-utils should be able to work with that. What do you think?

package has been renamed: https://github.com/kubernetes/utils/tree/master/mount

The new package also includes a change to drop consistentread calls on
newer kernels which don't require that workaround anymore:
kubernetes/mount-utils@4ae857e

Signed-off-by: Henning Surmeier <h.surmeier@mittwald.de>
@hensur
Copy link
Contributor Author

hensur commented Aug 8, 2024

Hi,
I rebased on latest develop. I also removed vendor, I based the PR on master at first, once I noticed that branch wasn't used anymore I rebased to develop. I forgot to remove vendor afterwards.

The newest mount-utils seem to require go 1.22:
https://github.com/kubernetes/mount-utils/blob/master/go.mod

@Abhinandan-Purkait
Copy link
Member

Abhinandan-Purkait commented Aug 9, 2024

This seems to be a flagged as an issue in the ci, can we use a version that works with 1.20? You probably need to bump the go versions in the github workflows.

@Abhinandan-Purkait
Copy link
Member

@hensur Any updates on this?

@Abhinandan-Purkait Abhinandan-Purkait requested a review from a team as a code owner October 13, 2025 16:53
@Abhinandan-Purkait
Copy link
Member

@hensur Thanks for the PR, the changes have been incorporated as a part of #678 We have included your commit in it. Closing this!

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.

3 participants