Skip to content

OVS: validate distributed VPC topology updates - #13792

Draft
Dogface2k wants to merge 2 commits into
apache:mainfrom
Dogface2k:agent/ovs-distributed-vpc-topology
Draft

OVS: validate distributed VPC topology updates#13792
Dogface2k wants to merge 2 commits into
apache:mainfrom
Dogface2k:agent/ovs-distributed-vpc-topology

Conversation

@Dogface2k

@Dogface2k Dogface2k commented Aug 4, 2026

Copy link
Copy Markdown

Summary

  • keep OVS distributed-router topology updates scoped to VPCs whose connectivity provider is OVS;
  • reject malformed VPC network topology data with actionable validation errors;
  • contain topology callback failures so one malformed or unavailable VPC does not abort updates for other VPCs;
  • add focused unit coverage for provider ownership, broadcast-key validation, missing gateway NICs, and callback failure handling.

Validation

The change is isolated from the NSX, VPN, CKS, and UI feature work. The focused OVS tests are included in this branch and the full 4.23 build has already compiled the corresponding source tree. Draft status is intentional pending the repository CI run / e2e

TODO: do not narrow the GRE key from long to signed int.

Keep greKey as long, parse it with Long.parseLong(), and explicitly validate it as a positive unsigned 32-bit GRE value. Integer.parseInt() rejects values from 2147483648 through 4294967295, even though GRE has a 32-bit key. OvsVpcPhysicalTopologyConfigCommand.Tier already stores greKey as long.

The test that currently expects 2147483648 to be rejected should be changed accordingly.

TODO: move isOvsDistributedRouterVpc(vpcId) inside the per-VPC try/catch.

The provider/VPC lookup currently happens before the catch, so an exception from that lookup can still abort processing of every later VPC, contrary to the failure-isolation goal of this PR.

Add a test where the first VPC's provider lookup throws and a second valid OVS distributed VPC still reaches topology generation or sending. The existing continuation tests only make the second VPC non-OVS and verify that it was inspected.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clgtm

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.21053% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.65%. Comparing base (4f11707) to head (42863fd).

Files with missing lines Patch % Lines
...va/com/cloud/network/ovs/OvsTunnelManagerImpl.java 84.21% 2 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               main   #13792    +/-   ##
==========================================
  Coverage     19.65%   19.65%            
+ Complexity    19792    19766    -26     
==========================================
  Files          6368     6368            
  Lines        574881   575296   +415     
  Branches      70351    70359     +8     
==========================================
+ Hits         112970   113099   +129     
- Misses       449639   449909   +270     
- Partials      12272    12288    +16     
Flag Coverage Δ
uitests 3.41% <ø> (ø)
unittests 20.93% <84.21%> (+<0.01%) ⬆️

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.

@DaanHoogland
DaanHoogland requested review from weizhouapache and a lite review from Copilot August 5, 2026 09:27
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

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.

3 participants