Perform resize of mounted volume if necessary #58794
Conversation
k8s-ci-robot
added
do-not-merge/release-note-label-needed
|
/sig storage |
k8s-ci-robot
added
the
sig/storage
k8s-ci-robot
added
release-note
There was a problem hiding this comment.
I see what you're trying to do, but IMO we should do resize even if a pod is Running - both ext4 and xfs allow online resize and ext3 allows it in most cases too. Why should user restart pods?
[note that ext3 without resize_inode option will fail also in this PR, the volume must be fully unmounted first]
pkg/util/resizefs/resizefs_linux.go
Outdated
| return false, deviceOpenErr | ||
| } | ||
|
|
||
| if deviceOpened { | ||
| deviceAlreadyOpenErr := fmt.Errorf("the device %s is already in use", devicePath) | ||
| return false, deviceAlreadyOpenErr | ||
| glog.Warningf("ResizeFS.Resize - Expanding mounted volume %s", devicePath) |
There was a problem hiding this comment.
Why it's Warning? It should be normal operation.
There was a problem hiding this comment.
can you please fix the level? I'm inclined to approve this PR then.
| deployment, err := framework.CreateDeployment(c, int32(1), map[string]string{"test": "app"}, nodeKeyValueLabel, ns, pvcClaims, "") | ||
| defer c.ExtensionsV1beta1().Deployments(ns).Delete(deployment.Name, &metav1.DeleteOptions{}) | ||
|
|
||
| By("Expanding current pvc") |
There was a problem hiding this comment.
Should you wait for a pod to be Running before resize? Expand controller may be faster than the pod.
There was a problem hiding this comment.
The CreateDeoployment function waits for a pod to be running.
|
@jsafrane proposal for implementing online resize is here - kubernetes/community#1535 , so we will get there. But there are design problems to be solved to get there. |
gnufied
force-pushed
the
perform-online-resize-if-mounted
branch
from
3fd411c to
85d17d0
Compare
|
@jsafrane also - I checked RHEL, Fedora 26 and 27, Ubuntu, Debian and Google COS - they all have |
Just throw a sensible event on the PVC that online resize failed with error from stderr. Let's hope it's enough to the users to either try offline resize or nag system admin to resize the volume manually (e.g. because is has errors). |
gnufied
force-pushed
the
perform-online-resize-if-mounted
branch
from
85d17d0 to
3e269eb
Compare
|
/test pull-kubernetes-unit |
|
The failure looks like unrelated flake or breakage - I have filed #58881 . I did not see an existing ticket for the failure. |
|
@jsafrane PTAL, changed log level of resize message. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
because some changes spilled into /assign @smarterclayton @brendandburns |
pkg/util/resizefs/resizefs_linux.go
Outdated
| @@ -84,7 +90,7 @@ func (resizefs *ResizeFs) Resize(devicePath string) (bool, error) { | |||
| } | |||
| return resizefs.xfsResize(devicePath) | |||
There was a problem hiding this comment.
I think the resize operation maybe can't work correctly for xfs file system: resizeFileSystem method is performed between attaching device and mounting device, it means the resize operation maybe performed before the device mounted. But xfs_growfs requires the device has been mounted. So if we try to resize a volume with xfs file system, we may receive an error event like this:
MountVolume.resizeFileSystem failed for volume "pvc-60265674-0406-11e8-9c91-0800273c9701" : resize of device /dev/rbd0 failed: exit status 1. xfs_growfs output: xfs_growfs: /dev/rbd0 is not a mounted XFS filesystem
There was a problem hiding this comment.
Thank you for catching this. It is fixed now.
gnufied
force-pushed
the
perform-online-resize-if-mounted
branch
from
3e269eb to
8beaa2f
Compare
k8s-github-robot
removed
the
lgtm
Add e2e test for mounted volume resize
gnufied
force-pushed
the
perform-online-resize-if-mounted
branch
from
8beaa2f to
afeb53e
Compare
|
@jsafrane PTAL. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
|
/approve |
|
/assign @smarterclayton |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: childsb, gnufied, jsafrane, smarterclayton The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-ci-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here. |
Under certain conditions - we must perform resize of volume even when it is mounted. This enables us to get around problem of resizing volumes used by deployments etc.