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

Allow overriding root disk offering & size, and expunge old root disk while restoring a VM #8800

Merged
merged 14 commits into from Apr 12, 2024

Conversation

vishesh92
Copy link
Member

@vishesh92 vishesh92 commented Mar 19, 2024

Description

This PR allows overriding root diskoffering id & size, and expunge old root disk while restoring VM. This is useful when you have to restore a VM with a new template but the newer template's size is smaller than the existing one.

New parameters for restoreVirtualMachine:

  1. diskofferingid - Override root volume's diskoffering
  2. rootdisksize - Override root volume's size (in GB)
  3. expunge - Option to expunge old volumes after restore
  4. details - Option to specify custom parameters for the selected disk offering

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale

  • Major
  • Minor

Screenshots (if appropriate):

image

How Has This Been Tested?

Tested Manually

@vishesh92
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 30.90%. Comparing base (0043540) to head (ec0790c).
Report is 12 commits behind head on 4.19.

Files Patch % Lines
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java 44.44% 9 Missing and 6 partials ⚠️
...e/cloudstack/api/command/user/vm/RestoreVMCmd.java 33.33% 0 Missing and 2 partials ⚠️
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.19    #8800      +/-   ##
============================================
- Coverage     30.99%   30.90%   -0.10%     
+ Complexity    34369    34272      -97     
============================================
  Files          5355     5355              
  Lines        376630   376715      +85     
  Branches      54807    54822      +15     
============================================
- Hits         116741   116412     -329     
- Misses       244535   244947     +412     
- Partials      15354    15356       +2     
Flag Coverage Δ
simulator-marvin-tests 24.72% <52.38%> (-0.14%) ⬇️
uitests 4.38% <ø> (-0.01%) ⬇️
unit-tests 16.57% <11.90%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vishesh92 vishesh92 changed the title Allow overriding root disk offering while restoring a VM Allow overriding root disk offering & size while restoring a VM Mar 19, 2024
@rohityadavcloud rohityadavcloud added this to the 4.19.1.0 milestone Mar 19, 2024
@blueorangutan
Copy link

Packaging result [SF]: ✖️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8970

@vishesh92
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8972

@vishesh92
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-9532)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42393 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8800-t9532-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@vishesh92
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@sureshanaparti sureshanaparti changed the title Allow overriding root disk offering & size while restoring a VM Allow overriding root disk offering & size, and expunge old root disk while restoring a VM Mar 26, 2024
Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

Code LGTM

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9040

@vishesh92
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-9585)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47382 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8800-t9585-kvm-centos7.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_02_trigger_shutdown Failure 336.76 test_safe_shutdown.py

@vishesh92 vishesh92 marked this pull request as ready for review March 27, 2024 08:27
@gpordeus
Copy link
Collaborator

Hi, @vishesh92, good change!

Could you describe your tests? Have you tried it with a compute offering with Disk offering strictness? What about expunge permissions?

@vishesh92
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9177

@blueorangutan
Copy link

[SF] Trillian test result (tid-9720)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50313 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8800-t9720-kvm-centos7.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 412.26 test_events_resource.py

@vishesh92
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-9740)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47010 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8800-t9740-kvm-centos7.zip
Smoke tests completed. 125 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 288.71 test_events_resource.py
test_01_events_resource Error 288.72 test_events_resource.py
test_04_deploy_vm_for_other_user_and_test_vm_operations Failure 87.23 test_network_permissions.py
ContextSuite context=TestNetworkPermissions>:teardown Error 1.36 test_network_permissions.py
test_create_pvlan_network Error 0.07 test_pvlan.py
test_01_redundant_vpc_site2site_vpn Failure 397.83 test_vpc_vpn.py

@sureshanaparti
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9189

@vishesh92
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9192

@vishesh92
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-9759)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47374 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8800-t9759-kvm-centos7.zip
Smoke tests completed. 127 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 292.22 test_events_resource.py
test_01_events_resource Error 292.23 test_events_resource.py
test_04_deploy_vm_for_other_user_and_test_vm_operations Failure 89.92 test_network_permissions.py
ContextSuite context=TestNetworkPermissions>:teardown Error 1.38 test_network_permissions.py

Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

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

LGTM, manually tested

@rohityadavcloud rohityadavcloud marked this pull request as ready for review April 12, 2024 12:17
@rohityadavcloud rohityadavcloud merged commit b998e7d into apache:4.19 Apr 12, 2024
5 of 24 checks passed
@rohityadavcloud rohityadavcloud deleted the add-disk-offering-restore-vm branch April 12, 2024 12:17
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request May 3, 2024
… while restoring a VM (apache#8800)

* Allow overriding root diskoffering id & size while restoring VM

* UI changes

* Allow expunging of old disk while restoring a VM

* Resolve comments

* Address comments

* Duplicate volume's details while duplicating volume

* Allow setting IOPS for the new volume

* minor cleanup

* fixup

* Add checks for template size

* Replace strings for IOPS with constants

* Fix saveVolumeDetails method

* Fixup

* Fixup UI styling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants