[cinder-csi-plugin] Verify filesystem size after resize #1543
Conversation
Signed-off-by: Andrey Klimentyev <[email protected]>
k8s-ci-robot
added
release-note
|
Hi @zuzzas. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
|
Build succeeded.
|
|
Build failed.
|
|
Build succeeded.
|
|
Build failed.
|
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
Nope, that won't work. Statfs() calculates available size without filesystem-specific. There are some insights here. Working on figuring this out... |
|
Build failed.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build failed.
|
| @@ -540,6 +541,17 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV | |||
| if _, err := r.Resize(devicePath, volumePath); err != nil { | |||
| return nil, status.Errorf(codes.Internal, "Could not resize volume %q: %v", volumeID, err) | |||
| } | |||
|
|
|||
| newFsSize, err := filesystem.GetFilesystemSize(volumePath) | |||
There was a problem hiding this comment.
You may want to use r.NeedResize() instead:
https://github.com/kubernetes/mount-utils/blob/master/resizefs_linux.go#L105
mount-utils is added in this PR: #1440, but it's interesting it doesn't exist in this file 🤔
It can also be used for #1539 as well :)
There was a problem hiding this comment.
I thought I've sent it as a comment, not a review! :D
There was a problem hiding this comment.
Aye, but the problem is. Due to the bug that I linked, the block device won't be resized as well. I think I'll add a check for the block device size; let's see how it works out. And if I can persuade maintainers to merge this.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
@zuzzas: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This config triggers a code path on NodeVolumeExpand() that works around the aforementioned bug. This PR has no purpose. |
What this PR does / why we need it:
Here is an issue that triggered a waterfall of "successful" Expands, which led to improper filesystem size: https://bugs.launchpad.net/openstack-ansible/+bug/1902914. Rescan does not work.
This means that in some cases, we can't trust a positive response from an API, and we should check an actual filesystem size after Expanding it.
Release note: