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

Fix listServiceOfferings regression #9894

Merged

Conversation

winterhazel
Copy link
Collaborator

Description

This PR fixes a regression introduced in 4.19.1.0 in the listServiceOfferings API.

The API's query was being built with an AND where there previously was an OR. This regression caused the API to not return the expected offerings, breaking the dynamic scale of instances via the UI (#9879).

Fixes: #9879

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)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

  1. I tried listing the service offering for a virtual machine with dynamic scale enabled that uses a custom constrained offering via CMK, and verified the performed query.

a) Before the regression, the following query would be performed:

SELECT service_offering_view.id, service_offering_view.uuid, service_offering_view.name, service_offering_view.display_text, service_offering_view.provisioning_type, service_offering_view.tags, service_offering_view.use_local_storage, service_offering_view.system_use, service_offering_view.cpu, service_offering_view.speed, service_offering_view.ram_size, service_offering_view.nw_rate, service_offering_view.mc_rate, service_offering_view.ha_enabled, service_offering_view.limit_cpu_use, service_offering_view.is_volatile, service_offering_view.host_tag, service_offering_view.default_use, service_offering_view.vm_type, service_offering_view.customized_iops, service_offering_view.min_iops, service_offering_view.max_iops, service_offering_view.hv_ss_reserve, service_offering_view.sort_key, service_offering_view.bytes_read_rate, service_offering_view.bytes_read_rate_max, service_offering_view.bytes_read_rate_max_length, service_offering_view.bytes_write_rate, service_offering_view.bytes_write_rate_max, service_offering_view.bytes_write_rate_max_length, service_offering_view.iops_read_rate, service_offering_view.iops_read_rate_max, service_offering_view.iops_read_rate_max_length, service_offering_view.iops_write_rate, service_offering_view.iops_write_rate_max, service_offering_view.iops_write_rate_max_length, service_offering_view.created, service_offering_view.removed, service_offering_view.domain_id, service_offering_view.domain_uuid, service_offering_view.domain_name, service_offering_view.domain_path, service_offering_view.zone_id, service_offering_view.zone_uuid, service_offering_view.zone_name, service_offering_view.deployment_planner, service_offering_view.cache_mode, service_offering_view.min_cpu, service_offering_view.max_cpu, service_offering_view.min_memory, service_offering_view.max_memory, service_offering_view.vsphere_storage_policy, service_offering_view.root_disk_size, service_offering_view.dynamic_scaling_enabled, service_offering_view.disk_offering_strictness, service_offering_view.disk_offering_id, service_offering_view.disk_offering_uuid, service_offering_view.disk_offering_name, service_offering_view.disk_offering_display_text, service_offering_view.encrypt_root FROM service_offering_view WHERE service_offering_view.disk_offering_strictness = 0  AND service_offering_view.use_local_storage = 0  AND  ( (service_offering_view.cpu >= 1 )  OR  (service_offering_view.cpu IS NULL  AND service_offering_view.max_cpu IS NULL )  OR  (service_offering_view.cpu IS NULL  AND service_offering_view.max_cpu >= 1 ) )  AND  (service_offering_view.speed >= 1024  OR service_offering_view.speed IS NULL )  AND  ( (service_offering_view.ram_size >= 1024 )  OR  (service_offering_view.ram_size IS NULL  AND service_offering_view.max_memory IS NULL )  OR  (service_offering_view.ram_size IS NULL  AND service_offering_view.max_memory >= 1024 ) )  AND service_offering_view.dynamic_scaling_enabled = 1  AND service_offering_view.system_use = 0  AND service_offering_view.removed IS NULL  ORDER BY service_offering_view.sort_key ASC , service_offering_view.id ASC  LIMIT 0, 500

Originally, the section that got regressed was:

(service_offering_view.ram_size IS NULL  AND service_offering_view.max_memory IS NULL )  OR  (service_offering_view.ram_size IS NULL  AND service_offering_view.max_memory >= 1024 )

b) With the regression, the following query is performed:

SELECT DISTINCT(service_offering.id) FROM service_offering  LEFT JOIN service_offering_details maxComputeDetailsSearch ON service_offering.id=maxComputeDetailsSearch.service_offering_id AND 'maxcpunumber'=maxComputeDetailsSearch.name  LEFT JOIN service_offering_details maxMemoryDetailsSearch ON service_offering.id=maxMemoryDetailsSearch.service_offering_id AND 'maxmemory'=maxMemoryDetailsSearch.name  INNER JOIN disk_offering diskOfferingSearch ON service_offering.disk_offering_id=diskOfferingSearch.id AND 'Active'=diskOfferingSearch.state WHERE service_offering.state = 'Active'  AND service_offering.disk_offering_strictness = 0  AND  ( service_offering.cpu >= 1  OR  ( service_offering.cpu IS NULL  AND  ( maxComputeDetailsSearch.value IS NULL  OR maxComputeDetailsSearch.value >= 1  )  )  )  AND  ( service_offering.speed IS NULL  OR service_offering.speed >= 1024  )  AND  ( service_offering.ram_size >= 1024  OR  ( service_offering.ram_size IS NULL  AND  ( maxMemoryDetailsSearch.value IS NULL  AND maxMemoryDetailsSearch.value >= 1024  )  )  )  AND service_offering.dynamic_scaling_enabled = 1  AND service_offering.system_use = 0  AND service_offering.removed IS NULL  AND  (diskOfferingSearch.use_local_storage = 0 ) ORDER BY service_offering.sort_key ASC , service_offering.id ASC  LIMIT 0, 500

The problematic section:

( service_offering.ram_size IS NULL  AND  ( maxMemoryDetailsSearch.value IS NULL  AND maxMemoryDetailsSearch.value >= 1024  )

c) With the patch:

SELECT DISTINCT(service_offering.id) FROM service_offering  LEFT JOIN service_offering_details maxComputeDetailsSearch ON service_offering.id=maxComputeDetailsSearch.service_offering_id AND 'maxcpunumber'=maxComputeDetailsSearch.name  LEFT JOIN service_offering_details maxMemoryDetailsSearch ON service_offering.id=maxMemoryDetailsSearch.service_offering_id AND 'maxmemory'=maxMemoryDetailsSearch.name  INNER JOIN disk_offering diskOfferingSearch ON service_offering.disk_offering_id=diskOfferingSearch.id AND 'Active'=diskOfferingSearch.state WHERE service_offering.state = 'Active'  AND service_offering.disk_offering_strictness = 0  AND  ( service_offering.cpu >= 1  OR  ( service_offering.cpu IS NULL  AND  ( maxComputeDetailsSearch.value IS NULL  OR maxComputeDetailsSearch.value >= 1  )  )  )  AND  ( service_offering.speed IS NULL  OR service_offering.speed >= 1024  )  AND  ( service_offering.ram_size >= 1024  OR  ( service_offering.ram_size IS NULL  AND  ( maxMemoryDetailsSearch.value IS NULL  OR maxMemoryDetailsSearch.value >= 1024  )  )  )  AND service_offering.dynamic_scaling_enabled = 1  AND service_offering.system_use = 0  AND service_offering.removed IS NULL  AND  (diskOfferingSearch.use_local_storage = 0 ) ORDER BY service_offering.sort_key ASC , service_offering.id ASC  LIMIT 0, 500

The second AND was reverted to an OR:

( service_offering.ram_size IS NULL  AND  ( maxMemoryDetailsSearch.value IS NULL  OR maxMemoryDetailsSearch.value >= 1024  )  )
  1. Via the UI, I tried to dynamic scale the VM, and verified that I was able to scale it successfully.

Screenshot from 2024-11-05 13-46-40

@winterhazel
Copy link
Collaborator Author

@blueorangutan package

@blueorangutan
Copy link

@winterhazel 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 Nov 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 15.11%. Comparing base (175eed2) to head (dd4c857).
Report is 23 commits behind head on 4.19.

Files with missing lines Patch % Lines
...ain/java/com/cloud/api/query/QueryManagerImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.19    #9894      +/-   ##
============================================
+ Coverage     15.08%   15.11%   +0.02%     
- Complexity    11203    11220      +17     
============================================
  Files          5404     5404              
  Lines        473423   473426       +3     
  Branches      59987    59988       +1     
============================================
+ Hits          71429    71550     +121     
+ Misses       394044   393878     -166     
- Partials       7950     7998      +48     
Flag Coverage Δ
uitests 4.30% <ø> (ø)
unittests 15.83% <0.00%> (+0.02%) ⬆️

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.

@blueorangutan
Copy link

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

Copy link
Member

@weizhouapache weizhouapache 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

not tested

Copy link
Contributor

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM, Tested manually with both Custom constrained and Custom unconstrained offerings

Before the fix

Screenshot 2024-11-07 at 4 13 07 PM

After the fix

Screenshot 2024-11-07 at 4 20 16 PM

@weizhouapache weizhouapache merged commit 03bdf11 into apache:4.19 Nov 7, 2024
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants