You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "winterhazel (via GitHub)" <gi...@apache.org> on 2024/11/05 19:25:25 UTC
[PR] Fix listServiceOfferings regression [cloudstack]
winterhazel opened a new pull request, #9894:
URL: https://github.com/apache/cloudstack/pull/9894
### 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)
- [X] 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
- [X] 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:
```sql
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 servic
e_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:
```sql
(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:
```sql
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.valu
e 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:
```sql
( service_offering.ram_size IS NULL AND ( maxMemoryDetailsSearch.value IS NULL AND maxMemoryDetailsSearch.value >= 1024 )
```
c) With the patch:
```sql
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.valu
e 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:
```sql
( service_offering.ram_size IS NULL AND ( maxMemoryDetailsSearch.value IS NULL OR maxMemoryDetailsSearch.value >= 1024 ) )
```
2. Via the UI, I tried to dynamic scale the VM, and verified that I was able to scale it successfully.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2486736497
@verendrakalsi this fix was not included in 4.19.1.3. It will be included in 4.19.2.0 and 4.20.0.0 onwards.
While 4.19.2.0 and 4.20.0.0 are not released, you can perform the dynamic scale of instances by directly calling the `scaleVirtualMachine` API through CloudMonkey.
```sh
(admin) 🐵 > scale virtualmachine id=<id_of_the_vm> serviceofferingid=<id_of_the_service_offering> details[0].cpuNumber=<number_of_cpus> details[0].memory=<memory_in_mbs>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "verendrakalsi (via GitHub)" <gi...@apache.org>.
verendrakalsi commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2486353518
I am using CloudStack 4.19.1.3 but still i have same issue. Do i still have to run all these queries in mysql ?
All these options are checked
1. enable enable.dynamic.scale.vm in global settings
2. Create a custom offering with dynamic scale enabled
3. Start an instance using this offering
4. Click the scale icon and pop-up with prompt will appear

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2457992832
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2458001404
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/9894?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 4.30%. Comparing base [(`175eed2`)](https://app.codecov.io/gh/apache/cloudstack/commit/175eed294c8d71697f26bac9caa7e2dcb598d9c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`dd4c857`)](https://app.codecov.io/gh/apache/cloudstack/commit/dd4c857695cdbc382563920d54ac319a6f884804?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1 commits behind head on 4.19.
> :exclamation: There is a different number of reports uploaded between BASE (175eed2) and HEAD (dd4c857). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (175eed2) | HEAD (dd4c857) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.19 #9894 +/- ##
============================================
- Coverage 15.08% 4.30% -10.79%
============================================
Files 5404 366 -5038
Lines 473423 29541 -443882
Branches 59987 5172 -54815
============================================
- Hits 71429 1272 -70157
+ Misses 394044 28125 -365919
+ Partials 7950 144 -7806
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9894/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9894/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.30% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9894/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/9894?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2458090695
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11511
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache merged PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listServiceOfferings regression [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #9894:
URL: https://github.com/apache/cloudstack/pull/9894#issuecomment-2457989925
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org