Skip to content

fix(hostnetwork): protect externally managed bridges from deletion - #671

Open
maiqueb wants to merge 1 commit into
openperouter:mainfrom
maiqueb:dont-delete-bridges-external-lifecycle
Open

fix(hostnetwork): protect externally managed bridges from deletion#671
maiqueb wants to merge 1 commit into
openperouter:mainfrom
maiqueb:dont-delete-bridges-external-lifecycle

Conversation

@maiqueb

@maiqueb maiqueb commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind cleanup
/kind feature
/kind design
/kind flake
/kind failing
/kind documentation
/kind regression
/kind example

What this PR does / why we need it:
deleteLinksForType extracts VNIs from bridge names and deletes bridges whose VNI is not in the configured set. External bridges encode a user-chosen number that differs from the L2VNI's actual VNI, causing wrongful deletion. Thread externalBridges set (map[string]struct{}) from API-level L2VNIs through the cleanup chain so both Linux and OVS paths skip bridges with External lifecycle. Drop the redundant OVS created-by external ID. Make lifecycle immutable via CEL XValidation.

Special notes for your reviewer:
Fixes: #662

Release note:

Fix external bridge deletion: bridges with External lifecycle are no longer wrongly removed by the reconciler when their name matches the br-hs-* pattern.

AI Guidelines Acknowledgment:

  • I have reviewed all changes in this PR, including any AI-generated content, and I take full responsibility for its accuracy and correctness.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved externally managed Linux and OVS bridges during VNI cleanup and underlay transitions.
    • Detached relevant virtual ports without deleting bridges managed outside the system.
    • Removed reliance on internal bridge metadata when identifying externally managed bridges.
    • Prevented changes to an L2 VNI’s lifecycle setting after it has been configured.
  • Tests
    • Added coverage confirming externally managed bridges are preserved while automatically managed bridges are removed.

deleteLinksForType extracts VNIs from bridge names and deletes bridges
whose VNI is not in the configured set. External bridges encode a
user-chosen number that differs from the L2VNI's actual VNI, causing
wrongful deletion. Thread externalBridges set (map[string]struct{})
from API-level L2VNIs through the cleanup chain so both Linux and OVS
paths skip bridges with External lifecycle. Drop the redundant OVS
created-by external ID. Make lifecycle immutable via CEL XValidation.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 414dba4f-69df-43fa-b612-b4dd97f4ccba

📥 Commits

Reviewing files that changed from the base of the PR and between ba65b89 and dbd2b6b.

📒 Files selected for processing (7)
  • api/v1alpha1/l2vni_types.go
  • internal/controller/routerconfiguration/host_config.go
  • internal/hostnetwork/l3vpn_test.go
  • internal/hostnetwork/ovs_bridge.go
  • internal/hostnetwork/vni.go
  • internal/hostnetwork/vni_ovs_bridge_test.go
  • internal/hostnetwork/vni_test.go

📝 Walkthrough

Walkthrough

The change makes L2VNI bridge lifecycle immutable, passes configured external bridge names into VNI cleanup, preserves external Linux and OVS bridges, and adds coverage for preserved and automatically managed bridges.

Changes

External bridge cleanup

Layer / File(s) Summary
Lifecycle validation and controller wiring
api/v1alpha1/l2vni_types.go, internal/controller/routerconfiguration/host_config.go
Bridge lifecycle fields cannot change after creation. Controller cleanup paths collect and pass configured external bridge names.
VNI cleanup preservation behavior
internal/hostnetwork/vni.go, internal/hostnetwork/ovs_bridge.go
VNI cleanup accepts external bridge sets, detaches related ports, and preserves matching Linux and OVS bridges. New OVS bridges no longer receive the created-by marker.
Cleanup behavior validation
internal/hostnetwork/*_test.go
Tests update cleanup calls and verify that external br-hs-* bridges remain while automatically managed bridges are deleted.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant host_config
  participant RemoveNonConfiguredVNIs
  participant LinuxBridgeCleanup
  participant OVSBridgeCleanup

  host_config->>host_config: collect configured external bridge names
  host_config->>RemoveNonConfiguredVNIs: pass external bridge set
  RemoveNonConfiguredVNIs->>LinuxBridgeCleanup: clean VNI links
  LinuxBridgeCleanup-->>RemoveNonConfiguredVNIs: preserve matching external bridges
  RemoveNonConfiguredVNIs->>OVSBridgeCleanup: clean VNI bridges
  OVSBridgeCleanup-->>RemoveNonConfiguredVNIs: preserve matching external bridges
Loading

Possibly related PRs

Suggested labels: kind/feature

Suggested reviewers: qinqon, fedepaol, andreaskaris

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: preventing deletion of externally managed bridges.
Description check ✅ Passed The description identifies the bug, explains the implementation, links issue #662, includes a release note, and acknowledges review responsibility.
Linked Issues check ✅ Passed The changes satisfy issue #662 by using External lifecycle data to preserve managed Linux and OVS bridges during cleanup.
Out of Scope Changes check ✅ Passed The API validation, OVS metadata removal, cleanup propagation, and tests directly support the linked issue and PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added kind/bug and removed kind/bug labels Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

L2VNI w/ hostMaster: deleteLinksForType deletes externally managed Linux bridges matching br-hs-* pattern

1 participant