fix guestOsMapper and move mapping code to latest upgrade #7095

DaanHoogland · 2023-01-13T16:31:56Z

Description

This PR fixes a buh in the guestos mapping facility and moves mappings, so current installations get the right mappings.

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?

upgraded from 4.16.0 and checked if the mappings where alright.
upgraded from 4.17 checked that mapping where alright and for the ubuntu on xenserver that an existing VM could still be stopped/started and migrated.

weizhouapache · 2023-01-13T18:50:33Z

it seems some codes in Upgrade41400to41500 needs to be moved as well.

$ grep -rl addGuestOsAndHypervisorMappings
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41400to41500.java
engine/schema/src/main/java/com/cloud/upgrade/GuestOsMapper.java

$ grep -rl updateGuestOsNameFromMapping
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java
engine/schema/src/main/java/com/cloud/upgrade/GuestOsMapper.java

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

DaanHoogland · 2023-01-16T09:13:48Z

@blueorangutan package

blueorangutan · 2023-01-16T09:14:03Z

@DaanHoogland 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 · 2023-01-16T13:05:19Z

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

DaanHoogland · 2023-01-16T13:05:52Z

@blueorangutan test

blueorangutan · 2023-01-16T13:07:03Z

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

nvazquez · 2023-01-16T16:34:19Z

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

@@ -69,8 +66,6 @@ public InputStream[] getPrepareScripts() {

    @Override
    public void performDataMigration(Connection conn) {
-        correctGuestOsNames(conn);
-        updateGuestOsMappings(conn);


I'm not sure about these schema changes, what would happen to templates/VMs created from existing guest OS on previous versions?

Outdated

blueorangutan · 2023-01-17T02:26:55Z

Trillian test result (tid-5868)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46615 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7095-t5868-kvm-centos7.zip
Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

codecov · 2023-01-17T14:49:02Z

Codecov Report

Merging #7095 (9f9e057) into main (5665782) will increase coverage by 0.00%.
The diff coverage is 0.27%.

❗ Current head 9f9e057 differs from pull request most recent head f83f8d4. Consider uploading reports for the commit f83f8d4 to get more accurate results

@@            Coverage Diff             @@
##               main    #7095    +/-   ##
==========================================
  Coverage     11.76%   11.77%            
- Complexity     7661     7666     +5     
==========================================
  Files          2503     2503            
  Lines        245958   246067   +109     
  Branches      38374    38380     +6     
==========================================
+ Hits          28946    28983    +37     
- Misses       213248   213313    +65     
- Partials       3764     3771     +7

Impacted Files	Coverage Δ
...src/main/java/com/cloud/upgrade/GuestOsMapper.java	`5.37% <0.00%> (-0.12%)`	⬇️
...ava/com/cloud/upgrade/dao/Upgrade41400to41500.java	`0.00% <ø> (ø)`
...ava/com/cloud/upgrade/dao/Upgrade41500to41510.java	`2.12% <ø> (+0.75%)`	⬆️
...ava/com/cloud/upgrade/dao/Upgrade41510to41520.java	`12.50% <ø> (-1.79%)`	⬇️
...ava/com/cloud/upgrade/dao/Upgrade41520to41600.java	`6.45% <ø> (-1.02%)`	⬇️
...ava/com/cloud/upgrade/dao/Upgrade41600to41610.java	`0.00% <ø> (ø)`
...ava/com/cloud/upgrade/dao/Upgrade41720to41800.java	`0.64% <0.27%> (-1.25%)`	⬇️
...m/resource/wrapper/LibvirtReadyCommandWrapper.java	`61.53% <0.00%> (-11.19%)`	⬇️
...dstack/network/contrail/model/ModelObjectBase.java	`21.15% <0.00%> (-7.70%)`	⬇️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java	`12.96% <0.00%> (-0.03%)`	⬇️
... and 11 more

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

DaanHoogland · 2023-01-18T10:13:47Z

@nvazquez @weizhouapache I added some test descriptions to the PR. Can you re-view?

weizhouapache

code lgtm

we need to verify the guest os mapping in environments below
(1) fresh 4.18
(2) upgrade from 4.17 to 4.18

I will verify when packages are done.

weizhouapache · 2023-01-18T15:50:51Z

@blueorangutan package

blueorangutan · 2023-01-18T15:51:03Z

@weizhouapache 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 · 2023-01-18T16:50:17Z

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

harikrishna-patnala

Code LGTM.

rohityadavcloud · 2023-01-19T10:33:13Z

LGTM @DaanHoogland would we need to do that with every release?

weizhouapache · 2023-01-19T10:34:45Z

LGTM @DaanHoogland would we need to do that with every release?

if we will do another 4.17 minor release (4.17.3.0), we need to fix in 4.17 as well.

DaanHoogland · 2023-01-19T11:43:02Z

@weizhouapache should I rebase on 4.17?

DaanHoogland · 2023-01-19T12:01:58Z

LGTM @DaanHoogland would we need to do that with every release?

Basically, no. Only because the mapping code was broken between 4.16.1 and 4.18 we need to redo the mapping in 4.18

weizhouapache · 2023-01-19T12:05:00Z

@weizhouapache should I rebase on 4.17?

@DaanHoogland

it will be complicated ...

you need to
(1) add upgrade path 4.17.2.0->4.17.3.0
(2) move everything to Upgrade41720to41730.java

and
what if there is not 4.17.3.0 release ?
what if 4.17.3.0 is released after 4.18.0.0, can upgrade from 4.17.3.0 to 4.18.0.0 ?
we may need to keep the codes in both files: Upgrade41720to41730.java and Upgrade41720to41800.java

DaanHoogland · 2023-01-19T12:36:04Z

@weizhouapache should I rebase on 4.17?

@DaanHoogland

it will be complicated ...

you need to
(1) add upgrade path 4.17.2.0->4.17.3.0
(2) move everything to Upgrade41720to41730.java

and
what if there is not 4.17.3.0 release ?
what if 4.17.3.0 is released after 4.18.0.0, can upgrade from 4.17.3.0 to 4.18.0.0 ?
we may need to keep the codes in both files: Upgrade41720to41730.java and Upgrade41720to41800.java

And merge forward guaranteed conflicts. Let's leave it as is.

sonarqubecloud · 2023-01-19T14:05:08Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
36 Code Smells

0.3% Coverage
88.7% Duplication

vladimirpetrov · 2023-01-20T10:11:51Z

@blueorangutan package

blueorangutan · 2023-01-20T10:13:03Z

@vladimirpetrov 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 · 2023-01-20T11:10:37Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 5338

blueorangutan · 2023-01-20T12:50:26Z

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

vladimirpetrov

LGTM based on manual testing.

BryanMLima · 2023-06-27T16:59:31Z

Hey @DaanHoogland, I know it has been a while, but did you check if there were duplicate Guest OS types after this change? I have some environments that were recently updated to the 4.18 from the 4.16 and there are 43 duplicate Guest OS types, specifically, those changed in this PR.

weizhouapache · 2023-07-28T01:40:32Z

@BryanMLima
yes, I have noticed it.
The method addGuestOsAndHypervisorMappings used in this PR will add a new guest OS (even it already exists) and then add guest os mappings.
I have created an issue to track it: #7783

DaanHoogland · 2023-07-28T07:30:53Z

@weizhouapache @BryanMLima , seems like we need;

a reconsiliation method (merging duplicates)
an extra check in addGuestOsAndHypervisorMappings to make sure such situations don't arise again

fix guestOsMapper and move mapping code to latest upgrade

d72dad8

DaanHoogland added this to the 4.18.0.0 milestone Jan 13, 2023

weizhouapache linked an issue Jan 13, 2023 that may be closed by this pull request

Xcp-ng 8.2.1 - issues reverting to vm snapshot #6941

Closed

weizhouapache mentioned this pull request Jan 13, 2023

Xcp-ng 8.2.1 - issues reverting to vm snapshot #6941

Closed

DaanHoogland commented Jan 15, 2023

View reviewed changes

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

DaanHoogland added 2 commits January 16, 2023 09:47

move remaining mappings after fix

f54e6ce

imports

2bf622d

guestOsMapper.updateGuestOsIdInHypervisorMapping calls moved

111846e

DaanHoogland marked this pull request as ready for review January 16, 2023 13:06

This comment was marked as outdated.

Sign in to view

nvazquez reviewed Jan 16, 2023

View reviewed changes

nvazquez self-requested a review January 16, 2023 16:35

apache deleted a comment from blueorangutan Jan 17, 2023

cleanup

8e54f07

add vmware 7 mappings

9f9e057

weizhouapache approved these changes Jan 18, 2023

View reviewed changes

harikrishna-patnala approved these changes Jan 19, 2023

View reviewed changes

minor sonar issues

cdf55c3

imports

f83f8d4

vladimirpetrov approved these changes Jan 20, 2023

View reviewed changes

DaanHoogland merged commit 2211182 into apache:main Jan 23, 2023

stephankruggg pushed a commit to scclouds/cloudstack that referenced this pull request Jan 25, 2023

fix guestOsMapper and move mapping code to latest upgrade (apache#7095)

419f7a9

weizhouapache mentioned this pull request Jul 27, 2023

Problem with guest os mappings for Red Hat Enterprise Linux 9 in ACS 4.18.0.0 #7781

Closed

weizhouapache mentioned this pull request Jul 28, 2023

Duplicated guest OSes in ACS 4.18.0.0 #7783

Closed

DaanHoogland deleted the guestOsMappingFix branch July 28, 2023 07:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix guestOsMapper and move mapping code to latest upgrade #7095

fix guestOsMapper and move mapping code to latest upgrade #7095

DaanHoogland commented Jan 13, 2023 •

edited

Loading

weizhouapache commented Jan 13, 2023

DaanHoogland commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

DaanHoogland commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

This comment was marked as outdated.

nvazquez Jan 16, 2023

blueorangutan commented Jan 17, 2023

codecov bot commented Jan 17, 2023 •

edited

Loading

DaanHoogland commented Jan 18, 2023

weizhouapache left a comment

weizhouapache commented Jan 18, 2023

blueorangutan commented Jan 18, 2023

blueorangutan commented Jan 18, 2023

harikrishna-patnala left a comment

rohityadavcloud commented Jan 19, 2023

weizhouapache commented Jan 19, 2023

DaanHoogland commented Jan 19, 2023

DaanHoogland commented Jan 19, 2023

weizhouapache commented Jan 19, 2023 •

edited

Loading

DaanHoogland commented Jan 19, 2023

sonarqubecloud bot commented Jan 19, 2023

vladimirpetrov commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

vladimirpetrov left a comment

BryanMLima commented Jun 27, 2023 •

edited

Loading

weizhouapache commented Jul 28, 2023

DaanHoogland commented Jul 28, 2023

fix guestOsMapper and move mapping code to latest upgrade #7095

fix guestOsMapper and move mapping code to latest upgrade #7095

Conversation

DaanHoogland commented Jan 13, 2023 • edited Loading

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

weizhouapache commented Jan 13, 2023

DaanHoogland commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

DaanHoogland commented Jan 16, 2023

blueorangutan commented Jan 16, 2023

This comment was marked as outdated.

nvazquez Jan 16, 2023

Choose a reason for hiding this comment

blueorangutan commented Jan 17, 2023

codecov bot commented Jan 17, 2023 • edited Loading

Codecov Report

DaanHoogland commented Jan 18, 2023

weizhouapache left a comment

Choose a reason for hiding this comment

weizhouapache commented Jan 18, 2023

blueorangutan commented Jan 18, 2023

blueorangutan commented Jan 18, 2023

harikrishna-patnala left a comment

Choose a reason for hiding this comment

rohityadavcloud commented Jan 19, 2023

weizhouapache commented Jan 19, 2023

DaanHoogland commented Jan 19, 2023

DaanHoogland commented Jan 19, 2023

weizhouapache commented Jan 19, 2023 • edited Loading

DaanHoogland commented Jan 19, 2023

sonarqubecloud bot commented Jan 19, 2023

vladimirpetrov commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

blueorangutan commented Jan 20, 2023

vladimirpetrov left a comment

Choose a reason for hiding this comment

BryanMLima commented Jun 27, 2023 • edited Loading

weizhouapache commented Jul 28, 2023

DaanHoogland commented Jul 28, 2023

DaanHoogland commented Jan 13, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading

weizhouapache commented Jan 19, 2023 •

edited

Loading

BryanMLima commented Jun 27, 2023 •

edited

Loading