Repository navigation
Conversation
Adds a 4.23.0 to 24.0.0 upgrade path (squashed from sb/upgradepath-424: engine-schema: upgrade path for 24.0.0, fix CS version, fix upgrade unit tests for cutover, fix imports).
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
Pearl1594
force-pushed
the
support-multi-vlan-guestnet
branch
from
September 14, 2026 19:57
711eb46 to
219f9cb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #14166 +/- ##
=============================================
- Coverage 20.45% 3.69% -16.77%
=============================================
Files 6487 494 -5993
Lines 589329 42821 -546508
Branches 72246 8122 -64124
=============================================
- Hits 120554 1581 -118973
+ Misses 455642 41010 -414632
+ Partials 13133 230 -12903
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 25.13% |
| Branch coverage | 20.24% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
The (nic_id, network_id) unique key rejected re-associating a network after a prior disassociate, since the old row's uniqueness survives as a soft-deleted (removed) row. Switch to a plain index so re-association after disassociation works, relying on application-level checks (not the DB) to reject a true duplicate live association.
…fallback Trunk NIC VLAN membership is delivered by reusing the existing guest bridge with vlan_filtering=1: the primary VLAN gets native/untagged membership, associated VLANs get tagged-only membership, via libvirt's <vlan trunk='yes'> element where supported, or manual `bridge vlan add` on older libvirt. Adds a live membership update path (UpdateNicVlanMembershipCommand/Answer + its KVM wrapper) so an association change on a running VM is pushed to the host without a restart, and a non-blocking diagnostic warning when the physical uplink is missing a VLAN a trunk NIC needs (CloudStack no longer manages the uplink's own VLAN membership - that is the operator's responsibility).
- New UsageEventUtils.publishNicNetworkOfferingUsageEvents() emits one usage event per network a nic bills against (primary + any trunk associations), replacing the old single-event publishUsageEvent at every NIC-billing call site - Ordinary non-trunk nics still produce exactly one event, unchanged - New nullable usage_network_offering.network_id column disambiguates rows when a trunk nic's networks share one network offering - Backfills network_id on a nic's own still-open usage row(s) the moment it first converts to a trunk nic; historic rows otherwise left alone
- New NicNetworkMapResponse + trunked/associatednetworks fields on NicResponse - Populated in listNics, listVirtualMachines, and the associate/disassociate/change-primary command responses - Trunk status derived from nic_network_map rows rather than a new column on the wide UserVmJoinVO view
- ASSOCIATED_NETWORKS/TRUNKED, needed by the previous commit's NicResponse fields
Core capability: - New associateNetworkToNic/disassociateNetworkFromNic/changeNicPrimaryNetwork admin APIs and associateNetworksToNic (programmatic, used by deploy) - A nic keeps one primary network plus any number of additional associated networks (nic_network_map), delivered as a VLAN trunk - changeNicPrimaryNetwork only allowed while the Instance is stopped - Rejects associating a network already reachable via another of the VM's nics, or with an overlapping CIDR to an existing association - Disassociating a network is blocked while an active PF/Static NAT/LB rule targets its allocated IP PF/Static NAT/LB support for associated networks: - NetworkModel.getNicAndIpInNetwork resolves a nic/guest-ip pair via the nic's primary match or a trunk association, reused by RulesManagerImpl and LoadBalancingRulesManagerImpl so these rules can target a trunk nic's associated networks, not just its primary Deployment planning: - Hosts without VLAN filtering enabled are excluded up front for any VM with a multi-VLAN trunk nic, with a reactive fallback exclusion during start as a backstop - Removing a nic cleans up any trunk associations it held as primary; deleting a network is blocked while still associated as a secondary Metadata: - A trunk nic's associated-network VLAN tags are exposed to the guest via the metadata service (nic-vlan-mapping file), gated by a new account-scoped config key, off by default
- EVENT_NIC_NETWORK_ASSOCIATE/DISASSOCIATE/PRIMARY_NETWORK_CHANGE, needed by the previous commit's @actionevent annotations
- New nicnetworkslist deploy param (nicnetworkslist[N].networkids, first id primary) lets a VM be deployed with trunk nics directly, mutually exclusive with networkids/iptonetworklist/vApp nicnetworklist - Also carries per-entry ip4address/ip6address for the primary and ip4addresses/ip6addresses (comma-separated) for its associated networks - a network with no requested IP auto-allocates - VNF: rejects a management-device nic from being requested as a trunk, both at deploy time and via the live associate API
- checkNoActiveRulesOnAssociation used findByIpAndNetworkId, which matches on a public IP's own address - the association's IP is a guest IP, so this never matched and the guard silently let disassociation through even with an active static NAT rule - Use findByAssociatedVmIdAndVmIp instead, which matches the actual static NAT target mapping
- validateVnfApplianceTrunkNics only ran at deploy time (nicnetworkslist) - the live associateNetworkToNic/associateNetworksToNic path had no equivalent check, so a VNF's management nic could be trunked after deploy - New single-nic validateVnfApplianceTrunkNic, called from associateNetworksInternal so both entry points are covered - No-op for any non-VNF template
- changeNicPrimaryNetwork updated networkId and the IPv4/IPv6 address but left gateway, netmask, broadcast/isolation uri, and IPv6 gateway/cidr pointing at the old primary network - wrong indefinitely, since nothing else ever recomputes them - New applyNetworkAddressingToNic refreshes them from the new primary network, mirroring the field derivation used when a nic is first created
- disassociateNetworkFromNic now clears nics.multi_network once nic_network_map has no remaining rows for that nic - Without this, listNics/listVirtualMachines kept reporting trunked=true and the deployment planner kept restricting the VM to VLAN-filtering- capable hosts even after all associations were removed
- New CommandSetupHelper.createDhcpEntryCommand overload takes addressing explicitly instead of reading it off a NicVO, since a nic's own DB row only ever carries its primary network's IP/gateway - NetworkServiceImpl sends the entry directly to the associated network's router(s) on live associate/disassociate, bypassing the DhcpServiceProvider pipeline (which re-resolves the nic from the DB and so can never see anything but the primary IP) - UserVmManagerImpl.finalizeStart converges a nic's associated-network DHCP entries with its current nic_network_map rows on every VM start, covering associations made while the VM was stopped and cleaning up entries for associations removed while stopped - New NicNetworkMapDao.listRemovedByNicId to support that cleanup
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19662 |
Unmanage removes nic_network_map rows via unmanageNics/removeNic without closing their usage records, leaking billed network associations for networks CS no longer manages.
…ng for trunk nic VMs getHostsForMigrateVmWithStorage lacked the isHostReadyForMultiNetworkNics guard that migrateVirtualMachine already enforces via checkVmMigrationDestination, so an explicit call could proceed past volume prep and fail deep in BridgeVifDriver.plug, masking the real error with a storage-pool-not-found CloudRuntimeException.
Post-migration VLAN trunk membership / CLVM lock-claim failures were only logged at WARN, leaving operators with no signal that a trunk VM lost network connectivity after migration. Migration still reports success; no rollback.
associateNetworksInternal's check-then-act (reject if already associated, then insert into nic_network_map) was not atomic. Concurrent associateNetworkToNic calls on the same nic could both pass the check and both insert, duplicating the association, its IP, its DHCP entry, and its usage event. nic_network_map has no unique constraint on (nic_id, network_id) to catch this at the DB layer. Serialize with a per-nic lock (_nicDao.acquireInLockTable) around the check and insert.
buildResultingNetworkSet checked whether a requested network was already associated with the nic, or already on one of the Instance's other nics, before checking the caller's access to that network. That leaks the network's association state to a caller who has no permission on it. Look up the network and check access first; only then evaluate the already-associated / other-nic checks, whose error messages reference the network.
…ociate buildResultingNetworkSet checked _accountMgr.checkAccess(caller, ...), which short-circuits unconditionally for root admin. Since associateNetworkToNic is root-admin-only at the API layer, this check could never actually deny anything: admin could associate any account's network onto any other account's Instance as a trunk association, with no ownership check at all. addNicToVirtualMachine avoids this by checking the Instance owner's entitlement to the network via NetworkModel.checkNetworkPermissions, which respects the admin.is.allowed.to.deploy.anywhere setting instead of bypassing unconditionally. Do the same here: check the Instance owner's permissions on the requested network, not the caller's.
Collaborator
|
@blueorangutan package |
|
@RosiKyu a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19691 |
|
…ls on old libvirt
…in the same call fails IP allocation
…ith networkid via trunk nic
…t and block trunk network changes on vms with snapshots
…n migrateNetwork/migrateVpc moves a network
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for multi-vlan trunk NICs
doc PR: apache/cloudstack-documentation#689
https://cwiki.apache.org/confluence/spaces/CLOUDSTACK/pages/451972290/Multi+VLAN+trunk+NICs+support+in+KVM
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
Core trunk mechanism (KVM delivery)
Association lifecycle
Cross-VPC trunk associations
VM deploy and VNF
Network type coverage
DHCP correctness
Usage and billing
Migration
AssignVirtualMachine to another account
UI
How did you try to break this feature and the system with this change?