Static Routes with nexthop non-functional for private gateways by bhouse-nexthop · Pull Request #12859 · apache/cloudstack

bhouse-nexthop · 2026-03-18T18:27:53Z

Description

Static routes were only being added to the main routing table, but policy-based routing (PBR) is active on VPC routers. This caused traffic coming in from specific interfaces to not find the static routes, as they use interface-specific routing tables (Table_ethX).

This fix:

Adds a helper method to find which interface a gateway belongs to by matching the gateway IP against configured interface subnets
Modifies route add/delete operations to update both the main table and the appropriate interface-specific PBR table
Inserts iptables FORWARD rules for static routes that don't have a nexthop of a private gateway.
Uses existing CsAddress databag metadata to avoid OS queries
Handles both add and revoke operations for proper cleanup
Adds comprehensive logging for troubleshooting

Fixes #12857

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

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

See #12857 for the tested scenario

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

boring-cyborg · 2026-03-18T18:27:57Z

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
Be sure to read the CloudStack Coding Conventions.
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/

Static routes were only being added to the main routing table, but policy-based routing (PBR) is active on VPC routers. This caused traffic coming in from specific interfaces to not find the static routes, as they use interface-specific routing tables (Table_ethX). This fix: - Adds a helper method to find which interface a gateway belongs to by matching the gateway IP against configured interface subnets - Modifies route add/delete operations to update both the main table and the appropriate interface-specific PBR table - Uses existing CsAddress databag metadata to avoid OS queries - Handles both add and revoke operations for proper cleanup - Adds comprehensive logging for troubleshooting Fixes apache#12857

bhouse-nexthop · 2026-03-19T19:06:33Z

@weizhouapache can you approve the workflow runs and review this. I've just confirmed this fixes both of my issues.

DaanHoogland · 2026-03-30T08:39:46Z

@blueorangutan package

blueorangutan · 2026-03-30T08:42:04Z

@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 · 2026-03-30T09:46:32Z

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

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

weizhouapache

@bhouse-nexthop
overall lgtm, not tested tet
left two minor comments

sureshanaparti · 2026-04-15T12:33:00Z

@blueorangutan package

blueorangutan · 2026-04-15T12:34:04Z

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

sureshanaparti · 2026-04-15T13:04:04Z

@blueorangutan package

blueorangutan · 2026-04-15T13:06:05Z

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

weizhouapache

code lgtm

not tested yet

Copilot

Copilot wasn't able to review any files in this pull request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DaanHoogland · 2026-04-15T14:02:44Z

                                "-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
                                (self.dev, guestNetworkCidr, self.address['gateway'], self.dev)])

+        # Process static routes for this interface


@bhouse-nexthop , can we make the follwing code a method? i.e process_static_routes_for_Interface(…)

a vast majority of this block was just moved up outside of the if self.is_private_gateway():, I don't think it warrants a helper function tbh. Plus, the code as it is right now has been well tested, so we'd have to go through the validation process again.

If you feel really strongly about it I'll do it, but I'd rather not :)

I feel strongly but that is not absolute. I don’t think you should force yourself or that it is a requirement for merging (at all) or that you should do it if you feel it is a risk. I do think that such extractions make code more readable and maintainable, hence my question.

DaanHoogland · 2026-04-15T14:05:07Z

code looks good generally, what can we do in terms of testing, other than trusting @bhouse-nexthop ? @sureshanaparti @rajujith (cc @weizhouapache @winterhazel )

DaanHoogland · 2026-04-15T14:07:34Z

btw, seems we have merged lint-errors. the errors in https://github.com/apache/cloudstack/actions/runs/24455968567/job/71456964714?pr=12859 refer files not touched in this PR!

blueorangutan · 2026-04-15T14:19:55Z

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

DaanHoogland · 2026-04-15T15:10:11Z

@blueorangutan test

blueorangutan · 2026-04-15T15:12:04Z

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

blueorangutan · 2026-04-16T06:12:22Z

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

Test	Result	Time (s)	Test File
ContextSuite context=TestClusterDRS>:setup	`Error`	0.00	test_cluster_drs.py

boring-cyborg bot added component:networking component:virtual-router Python Warning... Python code Ahead! labels Mar 18, 2026

bhouse-nexthop changed the title ~~Fix static routes to be added to PBR tables in VPC routers~~ Static Routes with nexthop non-functional for private gateways Mar 18, 2026

bhouse-nexthop force-pushed the fix-static-route-pbr branch 2 times, most recently from f51b1ab to 1141a7a Compare March 18, 2026 18:35

bhouse-nexthop marked this pull request as ready for review March 18, 2026 19:03

bhouse-nexthop changed the base branch from main to 4.22 March 18, 2026 19:13

boring-cyborg bot added component:agent component:api component:build component:integration labels Mar 18, 2026

bhouse-nexthop changed the base branch from 4.22 to main March 18, 2026 19:14

bhouse-nexthop force-pushed the fix-static-route-pbr branch from 1141a7a to 8a60b80 Compare March 18, 2026 19:18

boring-cyborg bot added component:integration-test component:orchestration component:storage component:UI component:vmware component:vpc labels Mar 18, 2026

bhouse-nexthop changed the base branch from main to 4.22 March 18, 2026 19:18

DaanHoogland added this to the 4.22.1 milestone Mar 19, 2026

bhouse-nexthop force-pushed the fix-static-route-pbr branch 3 times, most recently from 4dc4ac3 to e102b98 Compare March 19, 2026 19:03

bhouse-nexthop force-pushed the fix-static-route-pbr branch from e102b98 to 16228ff Compare March 19, 2026 19:54

weizhouapache requested a review from Copilot March 19, 2026 20:00

rajujith moved this from Todo to In Progress in Apache CloudStack 4.22.1 Mar 26, 2026

DaanHoogland requested a review from Copilot March 30, 2026 11:36

Copilot AI reviewed Mar 30, 2026

View reviewed changes

weizhouapache requested a review from Copilot March 31, 2026 07:04

Copilot AI reviewed Mar 31, 2026

View reviewed changes

weizhouapache requested a review from Copilot April 2, 2026 06:24

Copilot AI reviewed Apr 2, 2026

View reviewed changes

weizhouapache reviewed Apr 14, 2026

View reviewed changes

Comment thread systemvm/debian/opt/cloud/bin/cs/CsHelper.py

Comment thread systemvm/debian/opt/cloud/bin/cs/CsStaticRoutes.py Outdated

sureshanaparti added the status:needs-testing label Apr 15, 2026

network matching grep fix, don't let 1.2.3.4/32 match 11.2.3.4/32

eeff4bb

sureshanaparti requested review from DaanHoogland, nvazquez and weizhouapache April 15, 2026 13:08

weizhouapache approved these changes Apr 15, 2026

View reviewed changes

weizhouapache requested a review from Copilot April 15, 2026 13:37

Copilot AI reviewed Apr 15, 2026

View reviewed changes

DaanHoogland reviewed Apr 15, 2026

View reviewed changes

Conversation

bhouse-nexthop commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

boring-cyborg bot commented Mar 18, 2026

Uh oh!

bhouse-nexthop commented Mar 19, 2026

Uh oh!

DaanHoogland commented Mar 30, 2026

Uh oh!

blueorangutan commented Mar 30, 2026

Uh oh!

blueorangutan commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sureshanaparti commented Apr 15, 2026

Uh oh!

blueorangutan commented Apr 15, 2026

Uh oh!

sureshanaparti commented Apr 15, 2026

Uh oh!

blueorangutan commented Apr 15, 2026

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

bhouse-nexthop Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Apr 15, 2026

Uh oh!

DaanHoogland commented Apr 15, 2026

Uh oh!

blueorangutan commented Apr 15, 2026

Uh oh!

DaanHoogland commented Apr 15, 2026

Uh oh!

blueorangutan commented Apr 15, 2026

Uh oh!

blueorangutan commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

bhouse-nexthop commented Mar 18, 2026 •

edited

Loading