Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode (#3900) #3901
Conversation
kubevirt-bot
added
dco-signoff: yes
|
Hi @gaohoward. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with 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-sigs/prow repository. |
|
[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 |
kubevirt-bot
added
release-note
|
/test all |
gaohoward
force-pushed
the
cnv-44140
branch
2 times, most recently
from
4942c4d to
3869e6a
Compare
gaohoward
force-pushed
the
cnv-44140
branch
2 times, most recently
from
af4abed to
6a01cfa
Compare
|
I am probably missing something, but didn't #3384 fix this particular issue? Or did I miss a particular case with that PR? |
Yeah I think it goes a different path. In this case it falls back to host-clone from smart-clone (supported by provisioner), and in that case it didn't take the fs overhead into account. |
|
Gotcha, okay, let me know if you need a review on the PR. |
Yes pleas review. Thanks @awels ! |
pkg/controller/clone/host-clone.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| realSourcePvcSizeRequest := sourcePvc.Spec.Resources.Requests[corev1.ResourceStorage] |
There was a problem hiding this comment.
This is the only thing I have some concern about, this works fine if the source volume mode is block, since we can just inflate the size of the source PVC to account for the overhead. But if the source volume mode is filesystem, it is likely the source is already inflated, and we don't want to inflate it again. I think in that case we should check if there is a datavolume associated with the PVC, and get the size from there.
There was a problem hiding this comment.
Thanks @awel. I think I see the point. I'll try correct it.
There was a problem hiding this comment.
@awels I've made some changes to improve it. Pls take a look. Thanks!
gaohoward
force-pushed
the
cnv-44140
branch
2 times, most recently
from
d00ad6c to
95f11c4
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
when cloning from block to filesystem volume mode (kubevirt#3900) When cloning from a source, the host cloner should check the requested size for the target tmp pvc if the tmp source pvc is of block volume mode and the target pvc is of filesystem volume mode because it should take filesystem overhead into consideration and enlarge the original request size accordingly before creating the tmp pvc. Because currently the check is missing the cloning will fail on validation. Signed-off-by: Howard Gao <[email protected]>
6 participants
When cloning from a source, the host cloner should check the requested size for the target tmp pvc if the tmp source pvc is of block volume mode and the target pvc is of filesystem volume mode because it should take filesystem overhead into consideration and enlarge the original request size accordingly
before creating the tmp pvc.
Because currently the check is missing the cloning will fail on validation.
What this PR does / why we need it:
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 #
Special notes for your reviewer:
Release note: