Skip to content

KVM: clean up persistent VXLAN network bridges on all hosts on delete - #14240

Open
MitchDrage wants to merge 4 commits into
apache:4.20from
MitchDrage:vxlan-persistent-cleanup
Open

KVM: clean up persistent VXLAN network bridges on all hosts on delete#14240
MitchDrage wants to merge 4 commits into
apache:4.20from
MitchDrage:vxlan-persistent-cleanup

Conversation

@MitchDrage

@MitchDrage MitchDrage commented Sep 24, 2026

Copy link
Copy Markdown

Description

Fixes #13966

Deleting a persistent VXLAN network left its bridge and VXLAN interface behind on every host that never ran a VM on it. Two changes were needed:

  1. Management server: networkMeetsPersistenceCriteria() only accepted the Vlan broadcast scheme, so CleanupPersistentNetworkResourceCommand was never sent for vxlan:// networks. It now accepts Vlan and Vxlan.
  2. KVM agent: BridgeVifDriver.deleteBr() always built the VLAN-style bridge name (br<pif>-<vni>), while VXLAN bridges are created as brvx-<vni>. Once the command was dispatched, the agent still found no bridge and reported success. It now deletes brvx-<vni> for VXLAN networks.

I've written this PR which #13968 had started on, but didn't fix the KVM side of the issue.

L2 persistent VXLAN networks now also get their bridges set up on all hosts at implement time, matching VLAN behaviour.

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
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

  • Unit tests: NetworkOrchestratorTest covers the persistence criteria for VLAN and VXLAN. I added some more testing to BridgeVifDriverTest for VLAN and VXLAN.
  • Real bridges: ran createVnetBr() and deleteBr() with the real modifyvxlan.sh/modifyvlan.sh in a privileged container. With the fix, both bridges are removed. Without it, brvx-5000 and vxlan5000 remain.
  • Simulator: advanced zone with VXLAN isolation and 4 hosts. Created and deleted a persistent Isolated network and a persistent L2 network. With the fix, CleanupPersistentNetworkResourceCommand reaches all 4 hosts for both. Without it, it reaches none.

Not yet tested end to end on physical KVM hosts.

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

  • Ran the new VXLAN tests against the unmodified code to confirm they fail there.
  • Confirmed VLAN behaviour is unchanged: the VLAN lifecycle test, the container run and the simulator run all show the same results before and after the change.
  • Ran the full test suites of both changed modules (cloud-engine-orchestration, 157 tests; cloud-plugin-hypervisor-kvm, 535 tests). All pass.
  • Checked that nothing subclasses NetworkOrchestrator or BridgeVifDriver, since two methods were made protected for testing.

Co-authored-by: @waterWang

@MitchDrage
MitchDrage changed the base branch from main to 4.20 September 24, 2026 11:35
@MitchDrage MitchDrage changed the title Vxlan persistent cleanup KVM: clean up persistent VXLAN network bridges on all hosts on delete Sep 24, 2026
@DaanHoogland

DaanHoogland commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan package

@MitchDrage

MitchDrage commented Sep 30, 2026

Copy link
Copy Markdown
Author

@DaanHoogland - Did the package task fail to run?

@DaanHoogland

DaanHoogland commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

blueorangutan commented Oct 1, 2026

Copy link
Copy Markdown

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

@blueorangutan

blueorangutan commented Oct 1, 2026

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.40%. Comparing base (8eeccdb) to head (33ac0b8).
⚠️ Report is 2 commits behind head on 4.20.

Files with missing lines Patch % Lines
...cloud/hypervisor/kvm/resource/BridgeVifDriver.java 66.66% 3 Missing and 1 partial ⚠️
...tack/engine/orchestration/NetworkOrchestrator.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.20   #14240      +/-   ##
============================================
+ Coverage     16.38%   16.40%   +0.02%     
- Complexity    13614    13646      +32     
============================================
  Files          5669     5669              
  Lines        501532   501554      +22     
  Branches      60922    60927       +5     
============================================
+ Hits          82153    82298     +145     
+ Misses       410172   410032     -140     
- Partials       9207     9224      +17
Flag Coverage Δ
uitests 4.16% <ø> (ø)
unittests 17.27% <68.75%> (+0.03%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@blueorangutan

blueorangutan commented Oct 2, 2026

Copy link
Copy Markdown

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

@DaanHoogland

DaanHoogland commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

blueorangutan commented Oct 2, 2026

Copy link
Copy Markdown

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

@blueorangutan

blueorangutan commented Oct 3, 2026

Copy link
Copy Markdown

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

Test Result Time (s) Test File

@DaanHoogland

DaanHoogland commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@Damans227 , is this good to go/ready for testing now? (cc @MitchDrage )

@Damans227

Damans227 commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

is this good to go/ready for testing now?

@DaanHoogland yes, the leftover multicast route is fixed now and the tests cover both cases. good to go for testing from my side.

@DaanHoogland DaanHoogland added this to the 4.20.4 milestone Oct 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@Damans227 Damans227 Damans227 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component:kvm component:orchestration component:UI needs-testing

Projects

None yet

Milestone

4.20.4

Development

See More

Successfully merging this pull request may close these issues.

VXLAN persistent networks create bridges on hosts that never ran a VM on them, but don't clean them up

4 participants

@MitchDrage @DaanHoogland @blueorangutan @Damans227