Fix SQL query for uuid wrong format

[nvazquez]

Description

Fixes: #6652

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?

[acs-robot]

Found UI changes, kicking a new UI QA build
@blueorangutan ui

[blueorangutan]

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

[yadvr]

@nvazquez the question is - could it break on vmware too?
@blueorangutan package

[blueorangutan]

@rohityadavcloud 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]

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6654 (SL-JID-2171)

[nvazquez]

@rohityadavcloud the original fix was for Vmware, in some cases datastore clusters seem to get the UUID in a wrong format, but needs checking for other cases in Vmware in which the same thing can happen cc.@DK101010. In my testing of the original fix I was using Vmware but didn't get pools in the wrong uuid format, and this logic was not executed

[nvazquez]

Looks like in the case of Vmware + SolidFire it may be using an IQN as the UUID and this could also affect it

[blueorangutan]

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

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (4.17@1c7efcb). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             4.17    #6654   +/-   ##
=======================================
  Coverage        ?    5.87%           
  Complexity      ?     3936           
=======================================
  Files           ?     2454           
  Lines           ?   242669           
  Branches        ?    37977           
=======================================
  Hits            ?    14265           
  Misses          ?   226825           
  Partials        ?     1579           

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

[shwstppr]

@nvazquez cc @DK101010 is it possible to filter storage pools based on pool_type as #5382 mentions the issue is with datastore cluster on VMware?

[DK101010]

Hi @shwstppr @nvazquez
yes it is possible, pool_type = "DatastoreCluster".

We discussed at the time in which format uuids should be stored in the database and that it should always be the same format. That's why we created the sql back then.

Therefore, the code for the storage plugin and local storage should be adapted and not the uuids in the database should be used with the wrong format or am I missing something?

[nvazquez]

Thanks @DK101010 - I've updated the PR to restrict the SQL to datastore cluster only.
@blueorangutan package

[acs-robot]

Found UI changes, kicking a new UI QA build
@blueorangutan ui

[blueorangutan]

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

[slavkap]

Code LGTM

[shwstppr]

@blueorangutan package

[blueorangutan]

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

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6654 (SL-JID-2181)

[blueorangutan]

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

[nvazquez]

@blueorangutan test

[blueorangutan]

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

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

15.5% Coverage
0.1% Duplication

[shwstppr]

Hi @shwstppr @nvazquez yes it is possible, pool_type = "DatastoreCluster".

We discussed at the time in which format uuids should be stored in the database and that it should always be the same format. That's why we created the sql back then.

Therefore, the code for the storage plugin and local storage should be adapted and not the uuids in the database should be used with the wrong format or am I missing something?

@DK101010 I guess it is not just the code for the storage plugins and local storage. Like @slavkap pointed out in her issue it could be manually set or could be already set and used in production env where upgrades would fail. Using pool_type filter would be safer for now. Can you please review the changes?

[shwstppr]

Code LGTM

[DaanHoogland]

would this be particular to the concept of "DataCluster" or could there be implementations of the concept that handle things differenty @nvazquez @shwstppr ? (cc @harikrishna-patnala )

Reason for my question: it seems vmware specific but I don´t see a check for that in the code.

[nvazquez]

@DaanHoogland yes it seems particular to the Vmware implementation but its the only supported for the type 'DatastoreCluster'

[DaanHoogland]

@DaanHoogland yes it seems particular to the Vmware implementation but its the only supported for the type 'DatastoreCluster'

clear, i´d say it is maybe a bit early to amend this for a hypothetical future enhancement, just wondering. CLTGM

[DK101010]

I am a little confused.

I developed this because all storage (in VMware) uuids were stored with separators. But datastore cluster uuids was stored without separator.
It was said that all uuids are stored with separator in the database and that therefore the datastore cluster uuids are wrong.
Now it sounds like there is no structure to the storage uuids.

@shwstppr
if DatastoreClusters is only supported for VMware then code looks good.
If not, then I would add hypervisor= "VMware".

[blueorangutan]

Trillian test result (tid-4730)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40204 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6654-t4730-kvm-centos7.zip
Smoke tests completed. 100 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	Failure	588.75	test_kubernetes_clusters.py