Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resize disk image after cloning if we cloned a virtual disk. #938

Closed
wants to merge 1 commit into from

Conversation

awels
Copy link
Member

@awels awels commented Aug 22, 2019

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

What this PR does / why we need it:
Add the ability to resize a clone after the clone finishes and we have cloned a virtual disk image.

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 #930

Special notes for your reviewer:

Release note:

Resize cloned virtual disk images.

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M labels Aug 22, 2019
@awels awels requested a review from mhenriks August 22, 2019 20:16
@awels
Copy link
Member Author

awels commented Aug 22, 2019

Think we need a functional test for this? clone a 1G virtual disk into a 2G, the resulting virtual disk should have size 2G

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4005

  • 16 of 27 (59.26%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.03%) to 67.104%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/uploadserver/uploadserver.go 16 27 59.26%
Files with Coverage Reduction New Missed Lines %
pkg/uploadserver/uploadserver.go 1 75.26%
pkg/controller/upload-controller.go 5 64.49%
Totals Coverage Status
Change from base Build 4003: 0.03%
Covered Lines: 4804
Relevant Lines: 7159

💛 - Coveralls

return filesystemCloneProcessor(stream, common.ImporterVolumePath)
}

if contentType == BlockdeviceCloneContentType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeepers, I can't believe this was an empty if block. My intention was obviously to call blockdeviceCloneProcessor() which is now gone. Should I not be surprised that the data processor is doing the right thing? It's not going to use scratch space, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doing the right thing, and no it should not be using scratch space, since the pod didn't ask for it. Yeah I was boggled by the empty if, and started implementing it, but then I went, wait a minute, this works, we have tests that explicitly test this for block devices, and they work.

@awels
Copy link
Member Author

awels commented Aug 23, 2019

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 23, 2019
@awels
Copy link
Member Author

awels commented Aug 23, 2019

This won't work for snapshot cloning.

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2019
@kubevirt-bot
Copy link
Contributor

@awels: PR needs rebase.

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.

@awels
Copy link
Member Author

awels commented Oct 14, 2019

Need a different approach for all cloning options.

@awels awels closed this Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamically resize virtual disk image based on size of cloned PVC
4 participants