Skip to content

Fix ScaleVM to consider resize volume in any type of service offering - #7359

Merged
DaanHoogland merged 3 commits into
apache:4.17from
shapeblue:scaleVMfix
Apr 5, 2023
Merged

Fix ScaleVM to consider resize volume in any type of service offering#7359
DaanHoogland merged 3 commits into
apache:4.17from
shapeblue:scaleVMfix

Conversation

@harikrishna-patnala

@harikrishna-patnalaharikrishna-patnala commented Mar 23, 2023

Copy link
Copy Markdown
Member

Description

This PR fixes the issue #7325 where root disk is not getting considered for resize in some cases of ScaleVM operation.

While Scaling VM if the compute offering has the root disk size change (for any type of compute offering whether it is normal/constrained/unconstrained) CS should do the resize operation.

We already have a check here (

if (currentRootDiskOffering.getId() == newDiskOffering.getId() &&
(!newDiskOffering.isCustomized() || (newDiskOffering.isCustomized() && Objects.equals(rootVolumeOfVm.getSize(), rootDiskSizeBytes)))) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Volume %s is already having disk offering %s", rootVolumeOfVm, newDiskOffering.getUuid()));
}
continue;
}
) whether to skip the disk offering change or not. So here in this PR I've removed the checks if the compute offering is customized or not which I think irrelavant for root disk resize.

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 or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@harikrishna-patnala a Jenkins job has been kicked to build packages. It will be bundled with

SystemVM template(s). I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5764

@codecov

codecovBot commented Mar 23, 2023

Copy link
Copy Markdown

Codecov Report

Merging #7359 (e1c8b79) into 4.17 (dd51534) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@ Coverage Diff @@## 4.17 #7359 +/- ##
============================================
- Coverage 10.38% 10.38% -0.01% + Complexity 6651 6646 -5 
============================================
Files 2453 2453 Lines 242444 242438 -6 Branches 37941 37938 -3 ============================================
- Hits 25168 25166 -2 + Misses 214159 214154 -5 - Partials 3117 3118 +1 
Impacted FilesCoverage Δ
...n/java/com/cloud/storage/VolumeApiServiceImpl.java10.76% <0.00%> (-0.20%)⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@DaanHoogland

Copy link
Copy Markdown
Contributor

@harikrishna-patnala should this be based of the 4.17 branch?

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@harikrishna-patnala a Jenkins job has been kicked to build packages. It will be bundled with

SystemVM template(s). I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5767

@sonarqubecloud

Copy link
Copy Markdown

@github-advanced-security

Copy link
Copy Markdown

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@harikrishna-patnala a Jenkins job has been kicked to build packages. It will be bundled with

SystemVM template(s). I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5773

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@harikrishna-patnala a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_02_deploy_vm_with_extraconfig_kvmError1.17test_deploy_vm_extra_config_data.py
test_03_update_vm_with_extraconfig_kvmError0.16test_deploy_vm_extra_config_data.py
test_02_deploy_vm_on_specific_clusterError1.14test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_podError1.17test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_clusterError1.18test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_podError1.13test_vm_deployment_planner.py

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

@kiranchavala can you please test/review this ?

@kiranchavala

kiranchavala commented Mar 27, 2023

Copy link
Copy Markdown
Member

@harikrishna-patnala Tested the PR, and the resize of root disk is working fine

Change the Global setting "allow.diskoffering.change.during.scale.vm " to true

Create a custom compute offering with a custom disk offering
224026598-a0773ed4-d770-4a58-b236-a301759719c4

Scenarios

  1. Launched a vm a small offering
  2. Stop the vm
  3. scaled to a custom offering ( root disk resize worked fine)

  1. Launched to custom offering
  2. stop the vm
  3. scaled to the same custom offering (root disk resize worked fine)

@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_02_deploy_vm_with_extraconfig_kvmError1.15test_deploy_vm_extra_config_data.py
test_03_update_vm_with_extraconfig_kvmError1.13test_deploy_vm_extra_config_data.py
test_02_deploy_vm_on_specific_clusterError1.14test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_podError0.16test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_clusterError1.16test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_podError0.13test_vm_deployment_planner.py

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

all these failed tests, report "errorText:Service offering is inactive: " but this seems unrelated to my changes.

Anyone else see these failures elsewhere ?

@shwstppr

Copy link
Copy Markdown
Contributor

@harikrishna-patnala this seems an issue with 4.17 branch itself. I was seeing same failures here, #7313 (comment)

@weizhouapache

Copy link
Copy Markdown
Member

@harikrishna-patnala this seems an issue with 4.17 branch itself. I was seeing same failures here, #7313 (comment)

@harikrishna-patnala@shwstppr
the issue has been fixed in 4.18/main by #7183 and #7190

I think you can ignore the test failures with 4.17

@harikrishna-patnala

Copy link
Copy Markdown
MemberAuthor

thanks @shwstppr and @weizhouapache

@kiranchavala

kiranchavala commented Mar 29, 2023

Copy link
Copy Markdown
Member

@harikrishna-patnala

When a scale vm operation is performed in vmware 7. Resize of the root disk is failing on Vmare due to the following exception

com.cloud.utils.exception.CloudRuntimeException: Failed to resize volume operation of volume UUID: [c0e73278-3955-4981-91f6-0021dba430c1] due to - Failed to resize volume of VM [name: i-2-3-VM] due to: [Resize of volume in VM [name: i-2-3-VM] is not supported because Disk device [path: ROOT-3] has Parents: [6000C295-4f69-b62b-5d38-597886f37804].].

I have set vmware.create.full.clone to true

Could be related to #5992

Ignore the above exceptions

This issue is occurring without the PR changes

The PR LGTM

@kiranchavalakiranchavala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested the feature and its working fine, LGTM

Comment threadserver/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clgtm

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code lgtm

it fixes a valid bug

@apacheapache deleted a comment from blueorangutanApr 3, 2023
@apacheapache deleted a comment from blueorangutanApr 4, 2023
@shwstppr

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@shwstppr a 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
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5848

@shwstpprshwstppr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@DaanHoogland
DaanHoogland merged commit b2f1965 into apache:4.17Apr 5, 2023
@DaanHoogland
DaanHoogland deleted the scaleVMfix branch April 5, 2023 14:21
@DaanHoogland

Copy link
Copy Markdown
Contributor

@harikrishna-patnala this gives conflicts on merging forward. Can you have a look please?

DaanHoogland added a commit that referenced this pull request Apr 5, 2023
* 4.18:
Fix ScaleVM to consider resize volume in any type of service offering (#7359)
@weizhouapacheweizhouapache added this to the 4.18.1.0 milestone May 5, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@harikrishna-patnala@blueorangutan@DaanHoogland@github-advanced-security@kiranchavala@weizhouapache@shwstppr