fix(interface-manager): Do not delete interfaces we did not create

[daniel-noland]

The reconciler garbage collected every observed interface which was absent from the plan, exempting only interfaces of kinds it does not understand. The dataplane does not own the network namespace it runs in, so this destroyed other people's devices: flannel's vxlan device parses as an ordinary VTEP and its cni0 parses as an ordinary bridge, and both were removed on the first reconciliation pass, severing pod networking on the node.

Ownership is now decided by name. ManagedInterfaceName defines the dataplane's naming scheme (-vrf, -bri, -vtp, -tap) in one place, and both the code which generates those names and the code which recognizes them go through it, so the two cannot drift apart. An interface whose name does not fit the scheme is foreign: the reconciler leaves it strictly alone.

The rule is deliberately asymmetric. Failing to recognize one of our own interfaces leaks it, which is bounded and self correcting. Mistaking somebody else's interface for one of ours destroys their network, which is neither.

Foreign interfaces are also excluded from the observed vtep and vrf property indexes. Those indexes are keyed uniquely by vni and route table id in order to detect collisions between interfaces we manage, and a colliding foreign interface (flannel defaults to vni 1) evicted a legitimate observation of ours from the map, leaving the reconciler to conclude that its own interface was missing and to try, and fail, to recreate it until the config apply gave up.

The new integration test builds a flannel-like vxlan device and bridge in a private network namespace, gives a VPC the same vni flannel is using, and asserts that flannel's devices survive while a stale interface of ours is still collected.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

The PR adds a managed interface naming contract with kind-specific suffixes, centralizes VPC interface name generation, and updates dataplane observation and reconciliation to leave foreign interfaces untouched. Tests cover naming invariants, generated plans, foreign devices, and VPC reconciliation flows.

Managed interface ownership

Layer / File(s)	Summary
Managed name contract interface-manager/src/interface/managed.rs, interface-manager/src/interface/mod.rs	Adds managed interface kinds, constructors, conversions, classification, generators, tests, and public exports.
Centralized interface naming mgmt/src/processor/confbuild/namegen.rs, mgmt/src/vpc_manager/mod.rs	Routes VPC and tap interface naming through ManagedInterfaceName and ManagedInterfaceKind.
Ownership-aware observation and reconciliation mgmt/src/vpc_manager/mod.rs	Classifies owned interfaces, preserves observation collisions, and skips foreign interfaces during reconciliation.
Reconciliation validation and generated plans mgmt/src/vpc_manager/mod.rs, mgmt/tests/reconcile.rs, mgmt/Cargo.toml	Restricts generated plans to managed names and tests foreign-device preservation, stale owned-device removal, and updated VPC naming flows.

Suggested reviewers: qmonnet

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	It clearly matches the main change: preventing deletion of foreign interfaces by the interface manager.
Description check	✅ Passed	It accurately describes the ownership change, foreign-interface handling, and the new integration test.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

[Copilot]

Pull request overview

This PR prevents the interface reconciler from deleting network interfaces it did not create (e.g., CNI-managed devices like flannel’s flannel.1 and cni0) by introducing a single, shared naming scheme for “managed” interfaces and using it both for name generation and ownership recognition.

Changes:

• Added a centralized managed-interface naming/recognition module (ManagedInterfaceName / ManagedInterfaceKind) to determine dataplane ownership by name.
• Updated reconciliation and observation indexing in VpcManager to ignore foreign interfaces (both for garbage collection and for VNI/route-table collision detection maps).
• Added an integration test that creates flannel-like devices in a private network namespace and asserts they survive reconciliation while stale managed interfaces are removed.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
mgmt/tests/reconcile.rs	Adds an integration test (in a private netns) to ensure foreign CNI devices are not removed and don’t poison VNI-based indexing.
mgmt/src/vpc_manager/mod.rs	Updates observe/reconcile logic to treat only managed-name interfaces as eligible for GC and indexing; routes tap name creation through the shared naming scheme.
mgmt/src/processor/confbuild/namegen.rs	Routes VPC interface name generation through ManagedInterfaceName to keep generation and recognition consistent.
mgmt/Cargo.toml	Adds nix (sched feature) needed for unshare(CLONE_NEWNET) in the new integration test.
interface-manager/src/interface/mod.rs	Exposes the new managed-interface module via re-export.
interface-manager/src/interface/managed.rs	Introduces the managed-interface naming scheme, classification helpers, and associated tests/contracts.
Cargo.lock	Records the new nix dependency resolution.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

mgmt/tests/reconcile.rs:24

confidence: 9
tags: ["style"]

`AsyncSocket` is imported but never used in this test file; this will fail builds that treat warnings as errors (and adds noise otherwise). Remove the unused import.

use rekon::{Observe, Reconcile};
use rtnetlink::packet_route::link::{InfoData, InfoVxlan};
use rtnetlink::sys::AsyncSocket;
use rtnetlink::{LinkBridge, LinkVxlan};

</details>

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

[coderabbitai]

🧹 Nitpick comments (3)

mgmt/src/vpc_manager/mod.rs (1)

327-335: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Include the offending interface name in the error log.

error!("{e}") on line 332 drops the only context that makes the skip actionable — which config interface was dropped from the spec.

♻️ Proposed change

                     Err(e) => {
-                        error!("{e}");
+                        error!("cannot name a tap after {}: {e}", iface.name);
                         continue;
                     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mgmt/src/vpc_manager/mod.rs` around lines 327 - 335, Update the Err branch of
ManagedInterfaceName::new to include iface.name in the error! log alongside the
validation error, while preserving the existing continue behavior.

mgmt/tests/reconcile.rs (1)

263-274: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Tighten the flannel VTEP assertion to catch delete-and-recreate.

matches!(..., InterfaceProperties::Vtep(_)) only proves a vxlan device with that name exists. A reconciler that removed flannel.1 and created its own vxlan under a colliding name would still pass. Asserting the observed vni and port match flannel's (FLANNEL_VNI, FLANNEL_PORT) makes the test actually guard the property it describes on lines 125-127.

♻️ Proposed assertion

-        assert!(
-            matches!(flannel_vtep.properties, InterfaceProperties::Vtep(_)),
-            "{FLANNEL_VTEP} was replaced by something else: {flannel_vtep:?}"
-        );
+        match &flannel_vtep.properties {
+            InterfaceProperties::Vtep(props) => {
+                assert_eq!(props.vni.as_u32(), FLANNEL_VNI, "{FLANNEL_VTEP} was rebuilt");
+                assert_eq!(props.port.as_u16(), FLANNEL_PORT, "{FLANNEL_VTEP} was rebuilt");
+            }
+            other => panic!("{FLANNEL_VTEP} was replaced by something else: {other:?}"),
+        }

Adjust the accessor names to whatever VtepProperties exposes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mgmt/tests/reconcile.rs` around lines 263 - 274, Strengthen the flannel VTEP
assertion in the reconciliation test by matching the observed VTEP properties
against FLANNEL_VNI and FLANNEL_PORT, not merely InterfaceProperties::Vtep(_).
Use the actual accessor or field names exposed by VtepProperties, while
preserving the existing missing-interface and replacement diagnostics.

interface-manager/src/interface/managed.rs (1)

194-200: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Centralize managed-name truncation
InterfaceName is ASCII-only, so the slice here cannot hit a UTF-8 boundary panic. The remaining issue is duplicated truncation logic in interface-manager/src/interface/managed.rs and mgmt/src/vpc_manager/mod.rs; extract a shared helper on ManagedInterfaceName and use it from both sites.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@interface-manager/src/interface/managed.rs` around lines 194 - 200,
Centralize the duplicated managed-name truncation by adding a shared truncation
helper on ManagedInterfaceName, then update TypeGenerator::generate in
interface-manager/src/interface/managed.rs and the corresponding truncation site
in mgmt/src/vpc_manager/mod.rs (lines 556-568) to use it. Preserve the existing
MAX_BASE_LEN behavior and resulting name construction.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@interface-manager/src/interface/managed.rs`:
- Around line 194-200: Centralize the duplicated managed-name truncation by
adding a shared truncation helper on ManagedInterfaceName, then update
TypeGenerator::generate in interface-manager/src/interface/managed.rs and the
corresponding truncation site in mgmt/src/vpc_manager/mod.rs (lines 556-568) to
use it. Preserve the existing MAX_BASE_LEN behavior and resulting name
construction.

In `@mgmt/src/vpc_manager/mod.rs`:
- Around line 327-335: Update the Err branch of ManagedInterfaceName::new to
include iface.name in the error! log alongside the validation error, while
preserving the existing continue behavior.

In `@mgmt/tests/reconcile.rs`:
- Around line 263-274: Strengthen the flannel VTEP assertion in the
reconciliation test by matching the observed VTEP properties against FLANNEL_VNI
and FLANNEL_PORT, not merely InterfaceProperties::Vtep(_). Use the actual
accessor or field names exposed by VtepProperties, while preserving the existing
missing-interface and replacement diagnostics.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a7a24108-8931-4ad1-91e6-4e24ae25de29

📥 Commits

Reviewing files that changed from the base of the PR and between 86d41d5 and 9ee6ed9.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• interface-manager/src/interface/managed.rs
• interface-manager/src/interface/mod.rs
• mgmt/Cargo.toml
• mgmt/src/processor/confbuild/namegen.rs
• mgmt/src/vpc_manager/mod.rs
• mgmt/tests/reconcile.rs

[Frostman]

I've tested on VLAB and it seems to be fine and not touching the flannel-managed interfaces