Skip to content

Conversation

@GaOrtiga
Copy link
Contributor

@GaOrtiga GaOrtiga commented Sep 8, 2023

Description

While creating a VM using VMware, if the template does not have a nic adapter configured, CloudStack will default to E1000. There is a global configuration called VmwareSystemVmNicDeviceType that allows the operator to choose the default nic adapter for system VMs, however, no such configuration exists for user VMs.

A new global configuration was created to allow the operator to choose the default nic adapter for user VMs.

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

How Has This Been Tested?

I changed the value of the config to PCNet32 and verified that the VMs deployed after this change had the right nic adapter.

This step was repeated for all the options of the configuration.

@harikrishna-patnala
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@harikrishna-patnala 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.

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #7954 (97dcefb) into main (f6b2a58) will increase coverage by 15.07%.
Report is 55 commits behind head on main.
The diff coverage is 16.66%.

@@              Coverage Diff              @@
##               main    #7954       +/-   ##
=============================================
+ Coverage     14.40%   29.48%   +15.07%     
- Complexity    10116    30812    +20696     
=============================================
  Files          2748     5100     +2352     
  Lines        259424   367600   +108176     
  Branches      40385    56289    +15904     
=============================================
+ Hits          37378   108372    +70994     
- Misses       217210   244365    +27155     
- Partials       4836    14863    +10027     
Flag Coverage Δ
simulator-marvin-tests 25.68% <ø> (?)
uitests 4.86% <ø> (?)
unit-tests 14.40% <16.66%> (?)

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

Files Coverage Δ
...cloud/hypervisor/vmware/manager/VmwareManager.java 100.00% <100.00%> (ø)
...d/hypervisor/vmware/manager/VmwareManagerImpl.java 14.26% <0.00%> (ø)
...com/cloud/hypervisor/guru/VmwareVmImplementer.java 14.28% <0.00%> (-0.08%) ⬇️

... and 3580 files with indirect coverage changes

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

@blueorangutan
Copy link

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

@GaOrtiga
Copy link
Contributor Author

This looks good @GaOrtiga. Do we also need to use this setting in case of an Invalid NIC device type (in catch() in else case)

Thanks @harikrishna-patnala.

Yes, I think it makes sense to use it in that case, I will add a commit to implement it.

Copy link
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

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

CLGTM, did not test it

@DaanHoogland
Copy link
Contributor

@harikrishna-patnala is this lgty now?

Copy link
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM - didn't test it

Copy link
Member

@harikrishna-patnala harikrishna-patnala 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

@blueorangutan package

@blueorangutan
Copy link

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

@DaanHoogland
Copy link
Contributor

@blueorangutan test ol8 vmware-70u3

@blueorangutan
Copy link

@DaanHoogland [SL] unsupported parameters provided. Supported mgmt server os are: centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8, alma9. Supported hypervisors are: kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-alma9, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82

@DaanHoogland
Copy link
Contributor

@blueorangutan test alma8 vmware-70u3

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_01_browser_migrate_template Failure 5.43 test_image_store_object_migration.py
test_01_add_primary_storage_disabled_host Error 36.23 test_primary_storage.py
test_01_sys_vm_start Failure 0.09 test_secondary_storage.py
test_01_deploy_vm_on_specific_host Error 0.08 test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster Error 46.39 test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod Error 3601.58 test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster Error 0.13 test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod Error 4.42 test_vm_deployment_planner.py
test_03_live_migrate_VM_with_two_data_disks Error 61.82 test_vm_life_cycle.py
test_08_migrate_vm Error 0.06 test_vm_life_cycle.py
test_09_expunge_vm Failure 424.56 test_vm_life_cycle.py

@DaanHoogland
Copy link
Contributor

@blueorangutan test alma8 vmware-70u3 keepEnv

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_01_browser_migrate_template Failure 5.44 test_image_store_object_migration.py
test_02_balanced_drs_algorithm Failure 138.22 test_cluster_drs.py
test_03_deploy_and_destroy_VM_and_verify_network_resources_persist Error 0.04 test_persistent_network.py
test_01_add_primary_storage_disabled_host Error 37.25 test_primary_storage.py
test_01_sys_vm_start Failure 0.11 test_secondary_storage.py
test_01_deploy_vm_on_specific_host Error 0.09 test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster Error 16.69 test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod Error 3602.32 test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster Error 0.15 test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod Error 1.34 test_vm_deployment_planner.py
test_03_live_migrate_VM_with_two_data_disks Error 62.08 test_vm_life_cycle.py
test_08_migrate_vm Error 0.08 test_vm_life_cycle.py

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm, tested all four values and checked on vcenter what the adapter type would be.

@DaanHoogland DaanHoogland merged commit be4a648 into apache:main Nov 15, 2023
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 16, 2023
… for user VMs in VMware (apache#7954)

Co-authored-by: Gabriel <gabriel.fernandes@scclouds.com.br>
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.

6 participants