Fix VMware VM ingestion template selection and default template failure by nvazquez · Pull Request #8429 · apache/cloudstack

nvazquez · 2024-01-02T13:03:31Z

Description

This PR fixes the template selection regression for VMware Ingestion in the UI on 4.19.0 RC1 and adds back the default template selection for VMware

Fixes: #8428 #8432

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)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Unmanage VM -> Import VM

How did you try to break this feature and the system with this change?

nvazquez · 2024-01-02T13:03:50Z

@blueorangutan ui

codecov · 2024-01-02T13:11:52Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (9d4f071) 30.80% compared to head (09bcf41) 30.78%.

Files	Patch %	Lines
ui/src/views/tools/ImportUnmanagedInstance.vue	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8429      +/-   ##
============================================
- Coverage     30.80%   30.78%   -0.03%     
+ Complexity    33981    33958      -23     
============================================
  Files          5341     5341              
  Lines        374864   374865       +1     
  Branches      54518    54519       +1     
============================================
- Hits         115485   115384     -101     
- Misses       244114   244221     +107     
+ Partials      15265    15260       -5

Flag	Coverage Δ
simulator-marvin-tests	`24.68% <0.00%> (-0.04%)`	⬇️
uitests	`4.39% <0.00%> (ø)`
unit-tests	`16.46% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Pearl1594

LGTM

nvazquez · 2024-01-02T17:00:14Z

@blueorangutan ui

blueorangutan · 2024-01-02T17:01:03Z

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

blueorangutan · 2024-01-02T17:16:19Z

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8429 (QA-JID-249)

shwstppr

Code LGTM

harikrishna-patnala

code LGTM

DaanHoogland · 2024-01-03T07:47:41Z

ui/src/views/tools/ImportUnmanagedInstance.vue

                </a-select>
              </a-form-item>
-              <a-form-item name="templateid" ref="templateid" v-if="cluster.hypervisortype === 'KVM' && !selectedVmwareVcenter && !isDiskImport && !isExternalImport">
+              <a-form-item name="templateid" ref="templateid" v-if="cluster.hypervisortype === 'VMware' || (cluster.hypervisortype === 'KVM' && !selectedVmwareVcenter && !isDiskImport && !isExternalImport)">


this can be some kind of [].contains() structure, right?

Suggested change

<a-form-item name="templateid" ref="templateid" v-if="cluster.hypervisortype === 'VMware' || (cluster.hypervisortype === 'KVM' && !selectedVmwareVcenter && !isDiskImport && !isExternalImport)">

<a-form-item name="templateid" ref="templateid" v-if="['VMware', 'KVM'].contains(cluster.hypervisortype) && !selectedVmwareVcenter && !isDiskImport && !isExternalImport)">

?

Sorry this is not the same condition, we always want to display if for VMware but for KVM only if !selectedVmwareVcenter && !isDiskImport && !isExternalImport

I had the same question, thanks @nvazquez

ha yes thanks @nvazquez. sorry about my crooked eyes. Maybe you can extract the condition to a method or otherwise make it a little more reader friendly?

@nvazquez @DaanHoogland I'm merging this as it is as default template import is blocking testing for #8426 . Please open a new PR if the refactoring is needed here 🙏

DaanHoogland

clgtm

nvazquez · 2024-01-03T12:11:35Z

@shwstppr I've also added a fix for #8432 on this PR

nvazquez · 2024-01-03T12:11:44Z

@blueorangutan package

blueorangutan · 2024-01-03T12:12:03Z

@nvazquez 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.

Pearl1594

LGTM

blueorangutan · 2024-01-03T13:32:13Z

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

shwstppr · 2024-01-04T05:15:17Z

@blueorangutan test rocky8 vmware-70u3

blueorangutan · 2024-01-04T05:16:03Z

@shwstppr a [SL] Trillian-Jenkins test job (rocky8 mgmt + vmware-70u3) has been kicked to run smoke tests

shwstppr

Code LGTM

harikrishna-patnala

code LGTM

blueorangutan · 2024-01-04T18:51:23Z

[SF] Trillian test result (tid-8719)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47062 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8429-t8719-vmware-70u3.zip
Smoke tests completed. 120 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_balanced_drs_algorithm	`Error`	424.72	test_cluster_drs.py

shwstppr

LGTM. Tested VMware import

Template option visible in the UI now,

Import with default temporary template works fine,

import-vmware-default-template.mp4

…re (apache#8429) This PR fixes the template selection regression for VMware Ingestion in the UI on 4.19.0 RC1 and adds back the default template selection for VMware Fixes: apache#8428 apache#8432

UI: Fix VMware VM ingestion template selection

e9c82c6

nvazquez added this to the 4.19.0.0 milestone Jan 2, 2024

nvazquez requested review from Pearl1594, andrijapanicsb and shwstppr January 2, 2024 13:03

boring-cyborg bot added the component:UI label Jan 2, 2024

nvazquez requested review from DaanHoogland and yadvr January 2, 2024 13:03

Pearl1594 approved these changes Jan 2, 2024

View reviewed changes

shwstppr approved these changes Jan 3, 2024

View reviewed changes

harikrishna-patnala approved these changes Jan 3, 2024

View reviewed changes

DaanHoogland reviewed Jan 3, 2024

View reviewed changes

DaanHoogland approved these changes Jan 3, 2024

View reviewed changes

Fixes on the default template import

09bcf41

nvazquez mentioned this pull request Jan 3, 2024

VMware: VM import fails with default template #8432

Closed

nvazquez changed the title ~~UI: Fix VMware VM ingestion template selection~~ Fix VMware VM ingestion template selection and default template failure Jan 3, 2024

nvazquez requested review from DaanHoogland, Pearl1594, harikrishna-patnala and shwstppr January 3, 2024 12:13

Pearl1594 approved these changes Jan 3, 2024

View reviewed changes

shwstppr approved these changes Jan 4, 2024

View reviewed changes

harikrishna-patnala approved these changes Jan 4, 2024

View reviewed changes

shwstppr mentioned this pull request Jan 4, 2024

VMware: VM ingestion broken in case of dynamically handled hypervisor/guest OS versions. #8412

Closed

shwstppr approved these changes Jan 5, 2024

View reviewed changes

shwstppr merged commit 3f9dd4d into apache:main Jan 5, 2024

shwstppr deleted the fix-419rc1-vmingestion-regressions branch January 5, 2024 07:27

	<a-form-item name="templateid" ref="templateid" v-if="cluster.hypervisortype === 'VMware' \|\| (cluster.hypervisortype === 'KVM' && !selectedVmwareVcenter && !isDiskImport && !isExternalImport)">
	<a-form-item name="templateid" ref="templateid" v-if="['VMware', 'KVM'].contains(cluster.hypervisortype) && !selectedVmwareVcenter && !isDiskImport && !isExternalImport)">

Conversation

nvazquez commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

nvazquez commented Jan 2, 2024

Uh oh!

codecov bot commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Pearl1594 left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jan 2, 2024

Uh oh!

blueorangutan commented Jan 2, 2024

Uh oh!

blueorangutan commented Jan 2, 2024

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

nvazquez Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Jan 5, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jan 3, 2024

Uh oh!

nvazquez commented Jan 3, 2024

Uh oh!

blueorangutan commented Jan 3, 2024

Uh oh!

Pearl1594 left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 3, 2024

Uh oh!

shwstppr commented Jan 4, 2024

Uh oh!

blueorangutan commented Jan 4, 2024

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 4, 2024

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

nvazquez commented Jan 2, 2024 •

edited

Loading

codecov bot commented Jan 2, 2024 •

edited

Loading