Codecov Report

Attention: Patch coverage is 88.73239% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 30.83%. Comparing base (12f65fb) to head (63ab94a).
Report is 5 commits behind head on main.

@@             Coverage Diff              @@
##               main    #8654      +/-   ##
============================================
- Coverage     31.04%   30.83%   -0.21%     
+ Complexity    33902    33647     -255     
============================================
  Files          5404     5405       +1     
  Lines        380305   380371      +66     
  Branches      55506    55519      +13     
============================================
- Hits         118056   117298     -758     
- Misses       246496   247468     +972     
+ Partials      15753    15605     -148     

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.

@hsato03

I agree this setting should by dynamic, and it is better to be a zone-level setting. But, vpc does not belong to a cluster and vms in vpc might run with different hypervisors, in different clusters. IMHO, It does not make sense to be a cluster-level setting.

@weizhouapache

I agree this setting should by dynamic, and it is better to be a zone-level setting.

Since the maximum number of networks per VPC depends on the hypervisor where the VR is deployed (as pointed out in the description), I don't think it makes sense to change it to the zone level as the cluster is the highest possible structure to define a hypervisor.

But, vpc does not belong to a cluster and vms in vpc might run with different hypervisors, in different clusters.

The setting value is defined by the cluster in which the VPC's VR is running, using the lowest value found if the VPC has more than one VR and they are in different clusters (https://github.com/apache/cloudstack/pull/8654/files#diff-07bd71d9f58832d4429d7743f4887188a96aacc913dc48e0101470147ce42032R1893-R1922). Regarding and vms in vpc might run with different hypervisors, VMs running in different hypervisors do not impact in the VPC creation; therefore, does not make sense to consider it in the algorithm.

@weizhouapache

I agree this setting should by dynamic, and it is better to be a zone-level setting.

Since the maximum number of networks per VPC depends on the hypervisor where the VR is deployed (as pointed out in the description), I don't think it makes sense to change it to the zone level as the cluster is the highest possible structure to define a hypervisor.

But, vpc does not belong to a cluster and vms in vpc might run with different hypervisors, in different clusters.

The setting value is defined by the cluster in which the VPC's VR is running, using the lowest value found if the VPC has more than one VR and they are in different clusters (https://github.com/apache/cloudstack/pull/8654/files#diff-07bd71d9f58832d4429d7743f4887188a96aacc913dc48e0101470147ce42032R1893-R1922). Regarding and vms in vpc might run with different hypervisors, VMs running in different hypervisors do not impact in the VPC creation; therefore, does not make sense to consider it in the algorithm.

I can understand your code.
Your requirement is not clear to me. Can you explain more about the infrastructure? e.g. the hypervisor types, why the max vpc networks are different, what are the issues? Can it be a domain or account setting?

I can understand your code. Your requirement is not clear to me. Can you explain more about the infrastructure? e.g. the hypervisor types, why the max vpc networks are different, what are the issues? Can it be a domain or account setting?

@weizhouapache Each hypervisor can support a different number of network adapters. Comparing KVM and VMWare, VMWare defines a limited number of NICs for each ESXi machine (https://configmax.esp.vmware.com/guest?vmwareproduct=vSphere&release=vSphere%207.0&categories=1-0), while the number of tiers that can be allocated using KVM depends on the number of PCI slots availabe. For example, KVM provides 32 PCI slots, which are used to connect several devices, e.g. CD-ROM, keyboard, etc. Every ACS VR already consumes 9 slots of the 32 available; thus, in KVM we can have 23 slots for new tiers to be added.

Therefore, in an environment with KVM and VMware clusters under the same zone, applying the VMware limit to KVM is not interesting, as a VPC in KVM supports way more tiers than in VMware.

I will update the PR's description to make it clearer.

I can understand your code. Your requirement is not clear to me. Can you explain more about the infrastructure? e.g. the hypervisor types, why the max vpc networks are different, what are the issues? Can it be a domain or account setting?

@weizhouapache Each hypervisor can support a different number of network adapters. Comparing KVM and VMWare, VMWare defines a limited number of NICs for each ESXi machine (https://configmax.esp.vmware.com/guest?vmwareproduct=vSphere&release=vSphere%207.0&categories=1-0), while the number of tiers that can be allocated using KVM depends on the number of PCI slots availabe. For example, KVM provides 32 PCI slots, which are used to connect several devices, e.g. CD-ROM, keyboard, etc. Every ACS VR already consumes 9 slots of the 32 available; thus, in KVM we can have 23 slots for new tiers to be added.

Therefore, in an environment with KVM and VMware clusters under the same zone, applying the VMware limit to KVM is not interesting, as a VPC in KVM supports way more tiers than in VMware.

I will update the PR's description to make it clearer.

@hsato03
makes perfect sense.

However ...
the number of nics of VPC VRs contains

• 1 control NIC
• multiple guest NICs
• multiple public NICs

I would suggest you to create a setting for each hypervisor, e.g. virtual.machine.max.nics.kvm and virtual.machine.max.nics.vmware which consider all type of NICs, and check the value when

• add guest NIC to VR (of VPC and isolated networks, and even Shared networks)
• add public NICs to VR (of VPC and isolated networks, and even Shared networks)
• add NICs to user VMs

@hsato03
What do you think?
The limitations apply on all vms, including user vms and VRs.

@hsato03
makes perfect sense.

However ...
the number of nics of VPC VRs contains

• 1 control NIC
• multiple guest NICs
• multiple public NICs

I would suggest you to create a setting for each hypervisor, e.g. virtual.machine.max.nics.kvm and virtual.machine.max.nics.vmware which consider all type of NICs, and check the value when

• add guest NIC to VR (of VPC and isolated networks, and even Shared networks)
• add public NICs to VR (of VPC and isolated networks, and even Shared networks)
• add NICs to user VMs

@weizhouapache Thanks for your suggestion.

I agree that this situation should include VMs and VRs but the vpc.max.networks setting should still exist if the user wants to limit the number of tiers in a VPC regardless of the hypervisor. What do you think about delegating the limit checking of a hypervisor based on the settings you suggested and changing the scope of the vpc.max.networks to the account level in this case?

@weizhouapache Thanks for your suggestion.

I agree that this situation should include VMs and VRs but the vpc.max.networks setting should still exist if the user wants to limit the number of tiers in a VPC regardless of the hypervisor.

agree.

What do you think about delegating the limit checking of a hypervisor based on the settings you suggested and changing the scope of the vpc.max.networks to the account level in this case?

It looks good. Looking forward to your changes

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@blueorangutan package

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

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

@blueorangutan test alma9 kvm-alma9

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

[SF] Trillian test result (tid-9654)
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 56484 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8654-t9654-kvm-alma9.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

@blueorangutan test

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

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

@weizhouapache is this good for your sake now?

@blueorangutan package

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

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

@blueorangutan test

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

[SF] Trillian test result (tid-15361)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 52948 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8654-t15361-kvm-ol8.zip
Smoke tests completed. 147 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

@blueorangutan test

@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[SF] Trillian test result (tid-15581)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 53671 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8654-t15581-kvm-ol8.zip
Smoke tests completed. 147 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@hsato03 could you fix the conflicts here?

@blueorangutan package

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

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

@blueorangutan test

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

[SF] Trillian test result (tid-16449)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 49761 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8654-t16449-kvm-ol8.zip
Smoke tests completed. 150 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below: