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

server: fix mysql error when list Shared templates for projects #8020

Conversation

weizhouapache
Copy link
Member

Description

This fixes the mysql error when list Shared templates for projects

without this PR

(localcloud) 🐱 > list templates listall=true templatefilter=shared showunique=true projectid=-1
🙈 Error: (HTTP 530, error code 4250) DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: SELECT DISTINCT(template_view.id) FROM template_view WHERE template_view.account_type = 5  AND template_view.lp_account_id IN )  AND template_view.hypervisor_type='KVM' AND template_view.format != 'ISO'  AND template_view.template_state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress')  AND template_view.removed IS NULL  ORDER BY template_view.sort_key ASC , template_view.temp_zone_pair ASC  LIMIT 0, 500

with this PR, it returns empty result

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?

without this PR
```
(localcloud) 🐱 > list templates listall=true templatefilter=shared showunique=true projectid=-1
🙈 Error: (HTTP 530, error code 4250) DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: SELECT DISTINCT(template_view.id) FROM template_view WHERE template_view.account_type = 5  AND template_view.lp_account_id IN )  AND template_view.hypervisor_type='KVM' AND template_view.format != 'ISO'  AND template_view.template_state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress')  AND template_view.removed IS NULL  ORDER BY template_view.sort_key ASC , template_view.temp_zone_pair ASC  LIMIT 0, 500
```

with this PR, it returns empty result
@weizhouapache
Copy link
Member Author

screenshot
image

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SF] 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.

Comment on lines +3761 to 3764
if (permittedAccounts.isEmpty()) {
return new Pair<>(new ArrayList<>(), 0);
}
sc.addAnd("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
Copy link
Contributor

Choose a reason for hiding this comment

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

seems a bit like a hack on a hack, shouldn´t this be handled by the SearchCriteria<>.addAnd()?

Copy link
Member Author

Choose a reason for hiding this comment

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

@DaanHoogland
no idea. I just verified this change fixes the issue.
if you find another solution, it would be better, I will close this PR in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I researched a little and the handling of an empty set is going to be just as ugly. Let's go with this.

@blueorangutan
Copy link

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

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #8020 (925d181) into 4.18 (221f863) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               4.18    #8020      +/-   ##
============================================
- Coverage     13.06%   13.06%   -0.01%     
  Complexity     9106     9106              
============================================
  Files          2720     2720              
  Lines        257506   257508       +2     
  Branches      40149    40150       +1     
============================================
  Hits          33649    33649              
- Misses       219627   219629       +2     
  Partials       4230     4230              
Files Coverage Δ
...ain/java/com/cloud/api/query/QueryManagerImpl.java 3.03% <0.00%> (-0.01%) ⬇️

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

@DaanHoogland DaanHoogland added this to the 4.18.2.0 milestone Sep 29, 2023
@DaanHoogland DaanHoogland self-assigned this Sep 29, 2023
@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

[SF] Trillian test result (tid-7792)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 51877 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8020-t7792-kvm-centos7.zip
Smoke tests completed. 107 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_invalid_upgrade_kubernetes_cluster Failure 3615.62 test_kubernetes_clusters.py
test_02_upgrade_kubernetes_cluster Failure 3623.37 test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster Failure 0.05 test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster Failure 0.04 test_kubernetes_clusters.py
test_05_basic_lifecycle_kubernetes_cluster Failure 0.03 test_kubernetes_clusters.py
test_06_delete_kubernetes_cluster Failure 0.04 test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster Failure 0.04 test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster Failure 0.04 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.03 test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster Failure 48.67 test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown Error 113.11 test_kubernetes_clusters.py

Copy link
Contributor

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

@DaanHoogland
Copy link
Contributor

tested; as reported:
before:

(localcloud) 🐱 > list templates listall=true templatefilter=shared showunique=true projectid=-1
🙈 Error: (HTTP 530, error code 4250) DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: SELECT DISTINCT(template_view.id) FROM template_view WHERE template_view.account_type = 5  AND template_view.lp_account_id IN )  AND template_view.hypervisor_type='KVM' AND template_view.format != 'ISO'  AND template_view.template_state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress')  AND template_view.removed IS NULL  ORDER BY template_view.sort_key ASC , template_view.temp_zone_pair ASC  LIMIT 0, 500
(localcloud) 🐱 >  

after

(localcloud) 🐱 > list templates listall=true templatefilter=shared showunique=true projectid=-1
(localcloud) 🐱 >  

merging

@DaanHoogland DaanHoogland merged commit 3d8cc63 into apache:4.18 Oct 3, 2023
23 of 26 checks passed
@DaanHoogland DaanHoogland deleted the 4.18-fix-list-shared-templates-for-project branch October 3, 2023 08:45
DaanHoogland added a commit that referenced this pull request Oct 3, 2023
* 4.18:
  server: fix mysql error when list Shared templates for project (#8020)
shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Oct 12, 2023
…e#8020)

(cherry picked from commit 3d8cc63)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@DaanHoogland DaanHoogland removed their assignment Nov 16, 2023
Sign up for free to 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.

None yet

4 participants