fix mismatching between db uuids and custom attributes uuids #5382

DK101010 · 2021-08-27T10:39:45Z

Description

During the datastore cluster creation, cloudstack could not
recognize the existing primary storage and create a new one because
uuid format not equal.

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

Screenshots (if appropriate):

How Has This Been Tested?

Tested in my own Vmware env.

during the datastore cluster creation, cloudstack could not recognize the existing primary storage and create a new one because uuid format not equal

DaanHoogland

thought the code is not very bad, I don't think this is how the problem should be solved @DK101010 . Where the input does not adhere to the standard, it should be converted. not during the search. Don't you think?

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

DK101010 · 2021-08-27T13:12:33Z

thought the code is not very bad, I don't think this is how the problem should be solved @DK101010 . Where the input does not adhere to the standard, it should be converted. not during the search. Don't you think?

For sure, but I'm not sure why are uuids in custom attributes without and in db with separators. Also for this way we must update custom attributes of all storages and for this I can not estimate the impact of already existing implementation. Therefore this solution.

DaanHoogland · 2021-08-27T13:23:54Z

thought the code is not very bad, I don't think this is how the problem should be solved @DK101010 . Where the input does not adhere to the standard, it should be converted. not during the search. Don't you think?

For sure, but I'm not sure why are uuids in custom attributes without and in db with separators. Also for this way we must update custom attributes of all storages and for this I can not estimate the impact of already existing implementation. Therefore this solution.

Ok, so how about removing the dashes unconditionally but just for the comparison and otherwise keep using the originals?

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

rohityadavcloud · 2021-09-29T04:23:07Z

@blueorangutan package

blueorangutan · 2021-09-29T04:25:03Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-09-29T05:24:45Z

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

blueorangutan · 2021-09-29T05:24:45Z

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

DaanHoogland

minor nit, this is executed only during upgrade/install so not making a fuzz

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java

sureshanaparti · 2021-10-05T07:06:36Z

@blueorangutan package

rohityadavcloud · 2022-01-08T03:33:47Z

@blueorangutan package

blueorangutan · 2022-01-08T03:34:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2022-01-08T07:05:20Z

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

sureshanaparti · 2022-01-10T07:38:17Z

@blueorangutan test

nvazquez · 2022-02-07T03:33:06Z

@blueorangutan test

nvazquez · 2022-03-16T04:25:25Z

@blueorangutan test matrix

blueorangutan · 2022-03-16T04:26:03Z

@nvazquez a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2022-03-16T14:05:54Z

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

Test	Result	Time (s)	Test File

blueorangutan · 2022-03-16T14:39:11Z

Trillian test result (tid-3625)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35470 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3625-xenserver-71.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2022-03-16T14:57:46Z

Trillian test result (tid-3627)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36646 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3627-vmware-65u2.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java

nvazquez · 2022-03-17T02:34:29Z

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java

+                    updateStmt.setString(1, poolUuid.toString());
+                    updateStmt.addBatch();
+            }
+            updateStmt.executeBatch();


Sorry if not correct, but in case the result set is empty wouldn't this fail since the parameters are not set? Or does this method check the parameters before executing?

Hi @nvazquez, good point, no Idea how it works, I will check this.

I have checked it, executeBatch have no problems if no values are added.

nvazquez · 2022-03-23T16:47:43Z

@blueorangutan package

blueorangutan · 2022-03-23T16:48:03Z

@nvazquez 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 · 2022-03-23T17:23:18Z

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

nvazquez · 2022-03-25T14:47:37Z

@blueorangutan test

blueorangutan · 2022-03-25T14:48:03Z

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

blueorangutan · 2022-03-26T02:09:41Z

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

Test	Result	Time (s)	Test File

nvazquez · 2022-03-30T16:21:05Z

PIng for review @weizhouapache @Pearl1594 @DaanHoogland

DaanHoogland

code looks good

nvazquez · 2022-04-20T02:25:52Z

@blueorangutan package

nvazquez · 2022-04-20T02:25:52Z

@blueorangutan package

blueorangutan · 2022-04-20T02:27:03Z

@nvazquez 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 · 2022-04-20T03:22:23Z

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

nvazquez · 2022-04-20T03:32:45Z

@blueorangutan test

blueorangutan · 2022-04-20T03:33:03Z

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

blueorangutan · 2022-04-20T15:55:43Z

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

Test	Result	Time (s)	Test File
test_02_create_iso_with_checksum_sha256	`Error`	66.56	test_iso.py

nvazquez

LGTM - manually tested upgrade from 4.16.1 on Vmware env:

Before the upgrade:

MariaDB [cloud]> select id, uuid from storage_pool;
+----+--------------------------------------+
| id | uuid                                 |
+----+--------------------------------------+
|  2 | 0c4d8fb8-72b4-3c92-b06f-48310371df62 |
|  1 | ccd02aec-5e0a-323f-a358-d739fd5dd50c |
+----+--------------------------------------+
2 rows in set (0.00 sec)

MariaDB [cloud]> select * from version order by id desc limit 1;
+----+----------+---------------------+----------+
| id | version  | updated             | step     |
+----+----------+---------------------+----------+
| 37 | 4.16.1.0 | 2022-04-21 01:58:07 | Complete |
+----+----------+---------------------+----------+
1 row in set (0.00 sec)

Upgraded successfully:

...
2022-04-21 02:45:04,415 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- PR#5984 Create table to persist VM stats.
2022-04-21 02:45:04,415 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) DROP TABLE IF EXISTS `cloud`.`vm_stats` 
2022-04-21 02:45:04,418 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) CREATE TABLE `cloud`.`vm_stats` (   `id` bigint unsigned NOT NULL auto_increment COMMENT 'id',   `vm_id` bigint unsigned NOT NULL,   `mgmt_server_id` bigint unsigned NOT NULL,   `timestamp` datetime NOT NULL,   `vm_stats_data` text NOT NULL,   PRIMARY KEY (`id`)   ) ENGINE=InnoDB DEFAULT CHARSET=utf8 
2022-04-21 02:45:04,428 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- PR#5984 Update name for global configuration vm.stats.increment.metrics
2022-04-21 02:45:04,428 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) Update configuration set name='vm.stats.increment.metrics' where name='vm.stats.increment.metrics.in.memory' 
2022-04-21 02:45:04,431 DEBUG [c.c.u.d.Upgrade41610to41700] (main:null) (logid:) Replacement of faulty pool uuids
2022-04-21 02:45:04,448 INFO  [c.c.u.DatabaseUpgradeChecker] (main:null) (logid:) Cleanup upgrade Upgrade41610to41700 to upgrade from 4.16.1.0-4.17.0.0 to 4.17.0.0
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- Licensed to the Apache Software Foundation (ASF) under one
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- or more contributor license agreements.  See the NOTICE file
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- distributed with this work for additional information
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- regarding copyright ownership.  The ASF licenses this file
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- to you under the Apache License, Version 2.0 (the
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- "License"); you may not use this file except in compliance
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- with the License.  You may obtain a copy of the License at
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) --
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) --   http://www.apache.org/licenses/LICENSE-2.0
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) --
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- Unless required by applicable law or agreed to in writing,
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- software distributed under the License is distributed on an
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- KIND, either express or implied.  See the License for the
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- specific language governing permissions and limitations
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- under the License.
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) --;
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) -- Schema upgrade cleanup from 4.16.1.0 to 4.17.0.0
2022-04-21 02:45:04,451 DEBUG [c.c.u.d.ScriptRunner] (main:null) (logid:) --;
2022-04-21 02:45:04,452 DEBUG [c.c.u.DatabaseUpgradeChecker] (main:null) (logid:) Cleanup script Upgrade41610to41700 is executed successfully
2022-04-21 02:45:04,477 DEBUG [c.c.u.DatabaseUpgradeChecker] (main:null) (logid:) Upgrade completed for version 4.17.0.0
....

MariaDB [cloud]> select id, uuid from storage_pool;
+----+--------------------------------------+
| id | uuid                                 |
+----+--------------------------------------+
|  2 | 0c4d8fb8-72b4-3c92-b06f-48310371df62 |
|  1 | ccd02aec-5e0a-323f-a358-d739fd5dd50c |
+----+--------------------------------------+
2 rows in set (0.00 sec)

MariaDB [cloud]> select * from version order by id desc limit 1;
+----+----------+---------------------+----------+
| id | version  | updated             | step     |
+----+----------+---------------------+----------+
| 38 | 4.17.0.0 | 2022-04-21 02:45:04 | Complete |
+----+----------+---------------------+----------+
1 row in set (0.00 sec)

fix mismatching between db uuids and custom attributes uuids

a860143

during the datastore cluster creation, cloudstack could not recognize the existing primary storage and create a new one because uuid format not equal

DaanHoogland reviewed Aug 27, 2021

View reviewed changes

server/src/main/java/com/cloud/storage/StorageManagerImpl.java Outdated Show resolved Hide resolved

remove method call setUuid

1744bdd

nvazquez reviewed Aug 28, 2021

View reviewed changes

server/src/main/java/com/cloud/storage/StorageManagerImpl.java Show resolved Hide resolved

sureshanaparti reviewed Aug 30, 2021

View reviewed changes

server/src/main/java/com/cloud/storage/StorageManagerImpl.java Outdated Show resolved Hide resolved

DK101010 added 2 commits September 21, 2021 10:21

add upgrade step to fix faulty pool uuids

d869185

adapt method to transform uuid each time

acdc7b0

GutoVeronezi reviewed Sep 22, 2021

View reviewed changes

DK101010 added 3 commits September 23, 2021 07:25

extract error msg

4aac479

rm unused import

ab22868

resolve merge conflicts with main

5c54b0c

nvazquez added this to the 4.16.1.0 milestone Sep 23, 2021

nvazquez added the type:bug label Sep 23, 2021

add exception to log error as parameter

e52e7ab

DaanHoogland reviewed Sep 29, 2021

View reviewed changes

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java Outdated Show resolved Hide resolved

adapt sql to fetch wrong uuids

2f4ee05

rohityadavcloud modified the milestones: 4.16.1.0, 4.17.0.0 Nov 25, 2021

nvazquez reviewed Mar 17, 2022

View reviewed changes

move upgrade code to Upgrade41610to41700

a390dd2

DaanHoogland reviewed Apr 1, 2022

View reviewed changes

nvazquez approved these changes Apr 21, 2022

View reviewed changes

nvazquez requested review from Pearl1594 and weizhouapache April 21, 2022 02:51

DaanHoogland approved these changes Apr 21, 2022

View reviewed changes

weizhouapache approved these changes Apr 21, 2022

View reviewed changes

nvazquez merged commit 18d9c6d into apache:main Apr 21, 2022

slavkap mentioned this pull request Aug 17, 2022

KVM - Upgrade to main fails if the storage's UUID doesn't follow the UUID format #6652

Closed

shwstppr mentioned this pull request Aug 18, 2022

Fix SQL query for uuid wrong format #6654

Merged

12 tasks

fix mismatching between db uuids and custom attributes uuids #5382

fix mismatching between db uuids and custom attributes uuids #5382

Uh oh!

Conversation

DK101010 commented Aug 27, 2021

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DK101010 commented Aug 27, 2021

Uh oh!

DaanHoogland commented Aug 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rohityadavcloud commented Sep 29, 2021

Uh oh!

blueorangutan commented Sep 29, 2021

Uh oh!

blueorangutan commented Sep 29, 2021

Uh oh!

blueorangutan commented Sep 29, 2021

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sureshanaparti commented Oct 5, 2021

Uh oh!

rohityadavcloud commented Jan 8, 2022

Uh oh!

blueorangutan commented Jan 8, 2022

Uh oh!

blueorangutan commented Jan 8, 2022

Uh oh!

sureshanaparti commented Jan 10, 2022

Uh oh!

nvazquez commented Feb 7, 2022

Uh oh!

nvazquez commented Mar 16, 2022

Uh oh!

blueorangutan commented Mar 16, 2022

Uh oh!

blueorangutan commented Mar 16, 2022

Uh oh!

blueorangutan commented Mar 16, 2022

Uh oh!

blueorangutan commented Mar 16, 2022

Uh oh!

Uh oh!

nvazquez Mar 17, 2022

Choose a reason for hiding this comment

Uh oh!

DK101010 Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

DK101010 Mar 21, 2022

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Mar 23, 2022

Uh oh!

blueorangutan commented Mar 23, 2022

Uh oh!

blueorangutan commented Mar 23, 2022

Uh oh!

nvazquez commented Mar 25, 2022

Uh oh!

blueorangutan commented Mar 25, 2022

Uh oh!