Specify image size #522

awels · 2018-11-14T19:21:25Z

Combined code from #489 and #490 by
@gites and
@danielerez
Added some tests and rebased on current master.

What this PR does / why we need it:
This PR calculates the needed image size based on the PVC request size (Not the actual PVC size), and resizes the image to match.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #482

Special notes for your reviewer:
This should finally get the feature in CDI.

Release note:

Disk images are resized to the PVC request size.

Co-authored-by: Tomasz Pawelczak [email protected]
Co-authored-by: Daniel Erez [email protected]

awels · 2018-11-14T20:22:08Z

ci test please

awels · 2018-11-15T19:51:49Z

ci test please

awels · 2018-11-15T21:00:39Z

ci test please

awels · 2018-11-16T15:03:13Z

/hold

awels · 2018-11-19T14:48:49Z

ci test please

j-griffith · 2018-11-19T19:26:49Z

LGTM other than updating the comment to reflect the current direction on this... will merge after that's updated and CI passes.

awels · 2018-11-19T19:49:48Z

/hold cancel

pkg/util/util.go

Signed-off-by: Alexander Wels <[email protected]>

pkg/importer/dataStream.go

@gites

Combined code from PR#489 and PR#490 by @gites and @danielerez Added some tests and rebased on current master. Signed-off-by: Alexander Wels <[email protected]>

zvikorn · 2018-11-21T12:03:00Z

pkg/importer/dataStream.go

+// ResizeImage resizes the images to match the requested size. Sometimes provisioners misbehave and the available space
+// is not the same as the requested space. For those situations we compare the available space to the requested space and
+// use the smallest of the two values.
+func ResizeImage(dest, imageSize string) error {


@awels Is this being called somewhere?

Whoops, you are right it is not, but obviously it should, making a patch to fix now.

zvikorn · 2018-11-21T12:05:54Z

@awels where do we do the filesystem overhead calculation and subtract from requested size?

awels · 2018-11-21T13:33:50Z

@zvikorn ResizeImage does the calculation. The algorithm is: take whatever is passed from the controller (The request size) and then compare to the available space on the file system of the PVC that is mounted in the container. Pick the lesser of the two.

zvikorn · 2018-11-21T14:25:00Z

@awels As I need to use this algo as well, should we remove this from datastream over to util and make it common?

awels · 2018-11-21T14:27:55Z

I debated doing that, I wasn't sure if you could use the same algorithm or not, if you can, feel free.

…

On Wed, Nov 21, 2018 at 9:25 AM zvikorn ***@***.***> wrote: @awels <https://github.com/awels> As I need to use this algo as well, should we remove this from datastream over to util and make it common? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#522 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEUF7mEA9DYprPEoERTsY0u-xEC2Q6yPks5uxWI9gaJpZM4YeaWK> .

zvikorn · 2018-11-21T15:19:26Z

Yea..... I guess I only need to take the minimum between 'util.GetAvailableSpace(dest)' and t he requested size, so that's fine. Thanks! On Wed, Nov 21, 2018 at 4:28 PM Alexander Wels <[email protected]> wrote:

…

I debated doing that, I wasn't sure if you could use the same algorithm or not, if you can, feel free. On Wed, Nov 21, 2018 at 9:25 AM zvikorn ***@***.***> wrote: > @awels <https://github.com/awels> As I need to use this algo as well, > should we remove this from datastream over to util and make it common? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #522 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AEUF7mEA9DYprPEoERTsY0u-xEC2Q6yPks5uxWI9gaJpZM4YeaWK > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#522 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AcepjNKYqXAuz-ypI2TLyZYG-C3hia0uks5uxWLsgaJpZM4YeaWK> .

zvikorn · 2018-11-22T12:42:03Z

@awels so.... I cannot use the GetAvailableSpace as the PVC is not bound yet at that point. I will subtract 4% for FS from the requested image size, unless we have something else to think about...

awels · 2018-11-22T15:35:37Z

Not sure what you mean, in the importer the PVC is by definition bound, and that is where you have to do the calculation of which one to pick.

zvikorn · 2018-11-23T09:39:53Z

yea, I take my words back. I believe the PVC is bound at that point.
Anyway, I do not understand the connection between the GetAvailableSpace result and the FS overhead we need to take into account. Could you please elaborate about this a bit?
By the way, GetAvailableSpace returns '0' for me. Did it worked for you in the Resize patch? Did you verify the image size after resize is not '0'?

awels · 2018-11-23T15:05:08Z

So we don't have to take the FS overhead into account anymore, since its mounted with a FS on it, the available reported by the FS is what is available. We are just taking the minimum of that that value and the requested image size.

awels · 2018-11-23T16:00:38Z

Also just verified that I am getting the requested information from GetAvailableSpace, however found a bug that its not actually calling resize image at the right time, will have a PR to fix that soon.

zvikorn · 2018-11-23T20:11:12Z

Ok. Thanks. I tried getAvailableSpace in qemu.go in the createBlankImage method just right before I’m calling the qemu-img and it return ‘0’. I will try it again and also will try it in the importer.

…

Sent from my iPhone

On 23 Nov 2018, at 18:00, Alexander Wels ***@***.***> wrote: Also just verified that I am getting the requested information from GetAvailableSpace, however found a bug that its not actually calling resize image at the right time, will have a PR to fix that soon. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

zakkg3 · 2020-10-01T14:47:36Z

Also just verified that I am getting the requested information from GetAvailableSpace, however found a bug that its not actually calling resize image at the right time, will have a PR to fix that soon.

Is this fixed ?
i have a dataVolumeTemplates requesting 250Gi. the pvc is created with this size. then it clones the disk from a source pvc in another namespace (10G) but it never resizes the image. so my vm sees 10Gb instead of the 250...
Iam running cdi v1.19.0

awels · 2020-10-01T14:51:43Z

So this particular issue is talking about the virtual disk size. We do NOT resize the partition inside the VM because we don't know which (if there are multiple) partition you would like to have resized. So if you run something like fdisk inside the VM, it will show the disk is the size you requested, however the partitions will not be.

zakkg3 · 2020-10-01T15:02:40Z

Hi, thanks for the fast answer. I ve read your comment #1150 (comment) so i did but:

[root@vm-disk-big ~]# fdisk -l

Disk /dev/vda: 10.7 GB, 10737090560 bytes, 20970880 sectors

k get pvc 
vm-disk-big-disk        Bound    pvc-38b6ca99-d898-4bd9-8b96-5ffb6858a8d0   250Gi      RWX            netapp         123m

It seems its not resizing the virtual disk size..

k logs cdi-upload-vm-disk-big-disk -f
I1001 12:56:26.729816       1 uploadserver.go:63] Upload destination: /data/disk.img
I1001 12:56:26.729924       1 uploadserver.go:65] Running server on 0.0.0.0:8443
I1001 12:56:40.596917       1 uploadserver.go:319] Content type header is "filesystem-clone"
I1001 12:57:11.687584       1 util.go:174] begin untar to /data...
I1001 13:00:17.441136       1 uploadserver.go:338] Wrote data to /data/disk.img
I1001 13:00:17.441289       1 uploadserver.go:155] Shutting down http server after successful upload
I1001 13:00:17.942003       1 uploadserver.go:89] UploadServer successfully exited

awels · 2020-10-01T19:04:54Z

Oh i missed you were cloning, so there is a known issue #930 about resizing after cloning. In particular since we can clone any persistent volume, we cannot be sure we cloned a virtual disk. That is why we are not resizing after a clone. It is a definite issue we need to solve at some point.

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L labels Nov 14, 2018

awels requested review from mhenriks, aglitke, danielerez and j-griffith November 14, 2018 19:21

awels force-pushed the specify_image_size branch from 7cb7922 to 64da5a0 Compare November 14, 2018 20:58

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2018

awels force-pushed the specify_image_size branch from cf5af2a to 588116b Compare November 16, 2018 16:44

awels force-pushed the specify_image_size branch from 588116b to 035349e Compare November 19, 2018 18:05

gites mentioned this pull request Nov 19, 2018

Resize disk.img to actual DataVolume size #489

Closed

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 19, 2018

awels force-pushed the specify_image_size branch from 592c26e to 809135e Compare November 19, 2018 20:40

gites reviewed Nov 19, 2018

View reviewed changes

pkg/util/util.go Show resolved Hide resolved

awels force-pushed the specify_image_size branch from 809135e to 6c6331a Compare November 20, 2018 16:18

j-griffith approved these changes Nov 20, 2018

View reviewed changes

Add source and contentType annotations

db01ec3

Signed-off-by: Alexander Wels <[email protected]>

awels force-pushed the specify_image_size branch from cad870d to 10a2fc0 Compare November 20, 2018 19:26

aglitke reviewed Nov 20, 2018

View reviewed changes

pkg/importer/dataStream.go Outdated Show resolved Hide resolved

pkg/importer/dataStream.go Outdated Show resolved Hide resolved

Automatically resize image to fit the requested PVC.

260d4fa

Combined code from PR#489 and PR#490 by @gites and @danielerez Added some tests and rebased on current master. Signed-off-by: Alexander Wels <[email protected]>

awels force-pushed the specify_image_size branch from 10a2fc0 to 260d4fa Compare November 20, 2018 20:40

awels merged commit 546fdb4 into kubevirt:master Nov 20, 2018

awels mentioned this pull request Nov 20, 2018

[WIP] expand imported disk #490

Closed

zvikorn reviewed Nov 21, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify image size #522

Specify image size #522

awels commented Nov 14, 2018 •

edited

Loading

awels commented Nov 14, 2018

awels commented Nov 15, 2018

awels commented Nov 15, 2018

awels commented Nov 16, 2018

awels commented Nov 19, 2018

j-griffith commented Nov 19, 2018

awels commented Nov 19, 2018

zvikorn Nov 21, 2018 •

edited

Loading

awels Nov 21, 2018

zvikorn commented Nov 21, 2018

awels commented Nov 21, 2018 •

edited

Loading

zvikorn commented Nov 21, 2018

awels commented Nov 21, 2018 via email

zvikorn commented Nov 21, 2018 via email

zvikorn commented Nov 22, 2018

awels commented Nov 22, 2018

zvikorn commented Nov 23, 2018

awels commented Nov 23, 2018

awels commented Nov 23, 2018

zvikorn commented Nov 23, 2018 via email

zakkg3 commented Oct 1, 2020

awels commented Oct 1, 2020

zakkg3 commented Oct 1, 2020 •

edited

Loading

awels commented Oct 1, 2020

Specify image size #522

Specify image size #522

Conversation

awels commented Nov 14, 2018 • edited Loading

awels commented Nov 14, 2018

awels commented Nov 15, 2018

awels commented Nov 15, 2018

awels commented Nov 16, 2018

awels commented Nov 19, 2018

j-griffith commented Nov 19, 2018

awels commented Nov 19, 2018

zvikorn Nov 21, 2018 • edited Loading

Choose a reason for hiding this comment

awels Nov 21, 2018

Choose a reason for hiding this comment

zvikorn commented Nov 21, 2018

awels commented Nov 21, 2018 • edited Loading

zvikorn commented Nov 21, 2018

awels commented Nov 21, 2018 via email

zvikorn commented Nov 21, 2018 via email

zvikorn commented Nov 22, 2018

awels commented Nov 22, 2018

zvikorn commented Nov 23, 2018

awels commented Nov 23, 2018

awels commented Nov 23, 2018

zvikorn commented Nov 23, 2018 via email

zakkg3 commented Oct 1, 2020

awels commented Oct 1, 2020

zakkg3 commented Oct 1, 2020 • edited Loading

awels commented Oct 1, 2020

awels commented Nov 14, 2018 •

edited

Loading

zvikorn Nov 21, 2018 •

edited

Loading

awels commented Nov 21, 2018 •

edited

Loading

zakkg3 commented Oct 1, 2020 •

edited

Loading