create a bigger size volume from existing data source #258

Madhu-1 · 2019-03-13T09:06:04Z

if the newly requested volume is cloned from the existing snapshot, we need to check the request volume size is greater then the snapshot/parent-volume or not, if it is greater than the snapshot we need to
resize the requested volume to a new size.

Signed-off-by: Madhu Rajanna madhupr007@gmail.com

Logs after resizing the volume info

parent vol info

rbd image 'pvc-21c9a16f-456c-11e9-9814-525400dd375f':
	size 1 GiB in 256 objects
	order 22 (4 MiB objects)
	id: 8eb0643c9869
	block_name_prefix: rbd_data.8eb0643c9869
	format: 2
	features: layering
	op_features: 
	flags: 
	create_timestamp: Wed Mar 13 08:43:53 2019

resized volume info

rbd image 'pvc-63f2f902-456d-11e9-9814-525400dd375f':
	size 10 GiB in 2560 objects
	order 22 (4 MiB objects)
	id: 8f0a2ae8944a
	block_name_prefix: rbd_data.8f0a2ae8944a
	format: 2
	features: layering
	op_features: 
	flags: 
	create_timestamp: Wed Mar 13 08:52:54 2019
	parent: rbd/pvc-21c9a16f-456c-11e9-9814-525400dd375f@csi-rbd-pvc-21c9a16f-456c-11e9-9814-525400dd375f-snap-289d7f24-456c-11e9-a34f-0242ac11000e
	overlap: 1 GiB

if the newly requested volume is cloned from the existing snapshot, we need to check the request volume size is greater then the snapshot/parent-volume or not, if it is greater than the snapshot we need to resize the requested volume to a new size. Fixes: ceph#257 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 · 2019-03-13T09:06:23Z

@rootfs @gman0 PTAL

Madhu-1 · 2019-03-13T09:06:39Z

Fixes #257

rootfs · 2019-03-13T11:24:40Z

cc @j-griffith

rootfs · 2019-03-13T11:29:41Z

imo, the snap restore size is totally up to the storage backend, external provisioner does not need to revalidate the size. @xing-yang any input? thanks

Madhu-1 · 2019-03-13T13:49:09Z

@j-griffith can you please review this PR?

xing-yang · 2019-03-13T14:07:23Z

pkg/rbd/controllerserver.go

@@ -240,6 +240,10 @@ func (cs *ControllerServer) checkSnapshot(req *csi.CreateVolumeRequest, rbdVol *
 		return status.Error(codes.NotFound, err.Error())
 	}

+	if (rbdVol.VolSize * util.MiB) < rbdSnap.SizeBytes {


This check is not necessary in the CSI driver because it is already handled by the external-provisioner:
https://github.com/kubernetes-csi/external-provisioner/blob/master/pkg/controller/controller.go#L626

@xing-yang Thanks for the review, the intention of adding this check is am not sure all the CO will do this validation or not, to safeguard the failure I have added this check

@xing-yang is correct, it's not necessary in the Kubernetes context, the only thing I would mentoin however is that it is possible that at some point a different provisioner consumes the plugin. The check is redundant and unnecessary, but at the same time it doesn't really hurt either. Best case, it's never even hit, worst case we have a different CSI Provisioner some day and we miss this check somehow. Not a huge deal, just an observation.

Madhu-1 · 2019-03-13T14:16:02Z

imo, the snap restore size is totally up to the storage backend, external provisioner does not need to revalidate the size. @xing-yang any input? thanks

@xing-yang would really happy to get your input on this one.

xing-yang · 2019-03-13T14:23:21Z

imo, the snap restore size is totally up to the storage backend, external provisioner does not need to revalidate the size. @xing-yang any input? thanks

@xing-yang would really happy to get your input on this one.

@Madhu-1 In the provisioner check, we did say that the CSI plugin needs to handle expansion if volume size is bigger than snapshot size. So it depends on whether the plugin supports resize or not.
https://github.com/kubernetes-csi/external-provisioner/blob/master/pkg/controller/controller.go#L630

@gnufied can you take a look of the resize part?

j-griffith · 2019-03-13T14:28:11Z

#258 (comment)
@Madhu-1 In the provisioner check, we did say that the CSI plugin needs to handle expansion if volume size is bigger than snapshot size. So it depends on whether the plugin supports resize or not.
https://github.com/kubernetes-csi/external-provisioner/blob/master/pkg/controller/controller.go#L630

@gnufied can you take a look of the resize part?

I'd like to see what @gnufied says on this, but my opinion is that we should create the volume and then do the expansion for the user. It looks like we can do the expansion so we should just implement it that way.

gnufied · 2019-03-13T15:50:32Z

@j-griffith Is it enough to resize RBD volume? Is the volume's file system also expanded to accommodate new size or that must happen separately?

And gunfied ?? :D

Madhu-1 · 2019-03-18T04:48:47Z

@gnufied this is Volume resize of a cloned volume from the snapshot which is not mounted yet, yeah as this is just a PVC creation is should be enough I think

@j-griffith any comments?

humblec · 2019-03-18T12:31:00Z

Let me summarize this effort with a better summary:

First of all, this PR may be better titled to "create a bigger size volume from existing data source' instead of 'resize rbd volume'.

What the code do here is this

*) Clone/ create a volume from an existing source with bigger size and we reach there in 2 steps:

Create a clone volume from existing snapshot.
Increase the size of the volume to the requested size.

Hope this clear some dust. :) @xing-yang @gnufied @j-griffith

As a second thing, the question comes in, ie do we need "fs" expansion while we expand the volume?

My understanding is that, If the volume is mounted and used in pod, the expansion has to be performed on FS level. This need to be considered here @Madhu-1 .

@Madhu-1 , from code review pov, the check of the size can be outside snapshot function and it can be performed for any data source ie volume or snapshot and rest of the code can fall in.

if the requested volume size is less than the parent volume we should reject the request to avoid data loss. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 · 2019-03-21T11:27:08Z

@humblec updated PR PTAL

humblec · 2019-03-21T16:16:04Z

@Madhu-1 can you correct the Travis ?

gnufied · 2019-03-21T16:18:11Z

So, who does file system resizing in this case? The file system resizing still needs to happen and without that, expanded space won't be available to pod/workload. I don't see it being addressed.

Kubelet will not automatically expand the volume...

humblec · 2019-03-21T16:26:09Z

So, who does file system resizing in this case? The file system resizing still needs to happen and without that, expanded space won't be available to pod/workload. I don't see it being addressed.

@gnufied there are 2 choices, either we can address both data source (snapshot and volume ) here or address one which is snapshot data source. You are correct that we need fs expansion, but if we are only addressing snapshot data source via this PR, we are still good without fs expansion as snapsource is floating around without being attached to the pod.

gnufied · 2019-03-21T21:26:26Z

@humblec okay but what is the point of expanding the volume here when it won't be expanded when used by a pod? I think, we may have to handle this when a snapshot is restored to a larger volume. This can be done by setting pvc.Status.Capacity to snapshot size(smaller value) and pvc.Spec.Capacity to user requested size(larger value) and pv.Spec.Capacity to user requested value assuming - ceph returns the volume after expansion.

Above condition will automatically trigger expansion on the node. cc @xing-yang

nixpanic · 2020-04-16T16:13:14Z

@Madhu-1 please remove this from the v2.1.0 project and set the correct milestone (if any).

Madhu-1 · 2020-07-21T03:37:30Z

closing this one as its already addressed in #1244

Syncing latest changes from devel for ceph-csi

xing-yang reviewed Mar 13, 2019

View reviewed changes

Madhu-1 changed the title ~~resize rbd volume~~ create a bigger size volume from existing data source Mar 19, 2019

add validation for volume size

5b5e4f3

if the requested volume size is less than the parent volume we should reject the request to avoid data loss. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 force-pushed the fix-257 branch from aabe951 to 5b5e4f3 Compare March 21, 2019 11:26

humblec mentioned this pull request Oct 9, 2019

Release v2.0.0. of Ceph CSI driver. #557

Closed

12 tasks

humblec mentioned this pull request Jan 22, 2020

Release v2.1.0 of Ceph CSI driver. #806

Closed

14 tasks

humblec mentioned this pull request Mar 17, 2020

Release v3.0.0 of Ceph CSI driver #865

Closed

6 tasks

Madhu-1 closed this Jul 21, 2020

nixpanic pushed a commit to nixpanic/ceph-csi that referenced this pull request Mar 12, 2024

Merge pull request ceph#258 from red-hat-storage/sync_ds--devel

6d4b2fd

Syncing latest changes from devel for ceph-csi

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create a bigger size volume from existing data source #258

create a bigger size volume from existing data source #258

Madhu-1 commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019 •

edited

Loading

rootfs commented Mar 13, 2019

rootfs commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019

xing-yang Mar 13, 2019

Madhu-1 Mar 13, 2019

j-griffith Mar 13, 2019

Madhu-1 commented Mar 13, 2019

xing-yang commented Mar 13, 2019 •

edited

Loading

j-griffith commented Mar 13, 2019 •

edited

Loading

gnufied commented Mar 13, 2019 •

edited

Loading

Madhu-1 commented Mar 18, 2019

humblec commented Mar 18, 2019

Madhu-1 commented Mar 21, 2019

humblec commented Mar 21, 2019

gnufied commented Mar 21, 2019 •

edited

Loading

humblec commented Mar 21, 2019

gnufied commented Mar 21, 2019

nixpanic commented Apr 16, 2020

Madhu-1 commented Jul 21, 2020

create a bigger size volume from existing data source #258

create a bigger size volume from existing data source #258

Conversation

Madhu-1 commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019 • edited Loading

rootfs commented Mar 13, 2019

rootfs commented Mar 13, 2019

Madhu-1 commented Mar 13, 2019

xing-yang Mar 13, 2019

Choose a reason for hiding this comment

Madhu-1 Mar 13, 2019

Choose a reason for hiding this comment

j-griffith Mar 13, 2019

Choose a reason for hiding this comment

Madhu-1 commented Mar 13, 2019

xing-yang commented Mar 13, 2019 • edited Loading

j-griffith commented Mar 13, 2019 • edited Loading

gnufied commented Mar 13, 2019 • edited Loading

Madhu-1 commented Mar 18, 2019

humblec commented Mar 18, 2019

Madhu-1 commented Mar 21, 2019

humblec commented Mar 21, 2019

gnufied commented Mar 21, 2019 • edited Loading

humblec commented Mar 21, 2019

gnufied commented Mar 21, 2019

nixpanic commented Apr 16, 2020

Madhu-1 commented Jul 21, 2020

Madhu-1 commented Mar 13, 2019 •

edited

Loading

xing-yang commented Mar 13, 2019 •

edited

Loading

j-griffith commented Mar 13, 2019 •

edited

Loading

gnufied commented Mar 13, 2019 •

edited

Loading

gnufied commented Mar 21, 2019 •

edited

Loading