fix: Explicitly flush guest netns route/address on CNI DEL

internal/cni/cni_test.go

@@ -1233,6 +1233,50 @@ func TestCmdDelIdempotentMissingResources(t *testing.T) {
 	}
 }
 
+// TestCmdDelFlushesGuestNetnsConfig reproduces the production incident: a
+// hostNetwork pod's Multus secondary attachment resolves args.Netns to the
+// same namespace the guest link already lives in, so host-device DEL's
+// move-back-out-of-the-netns — and the kernel's implicit address/route
+// flush that only fires on a *real* namespace change — never happens.
+// cmdDel must flush the guest interface's address/default route itself
+// (flushGuestNetnsConfig), not rely solely on that side effect, or the next
+// ADD against the same interface wedges with "add default route ...: file
+// exists".
+func TestCmdDelFlushesGuestNetnsConfig(t *testing.T) {
+	requireRoot(t)
+
+	netnsPath, cleanup := createTestNetnsWithDummy(t)
+	defer cleanup()
+
+	if err := configureInterfaceInNetns(netnsPath, "test-dummy", testIPNet, testGateway, nil, nil); err != nil {
+		t.Fatalf("configureInterfaceInNetns: %v", err)
+	}
+
+	conf := fmt.Sprintf(
+		`{"cniVersion":"1.0.0","name":"test",`+
+			`"type":"galactic-cni","vpc":"%s",`+
+			`"vpcattachment":"%s","interface_type":"veth"}`,
+		testVPC, testAttachment,
+	)
+	args := &skel.CmdArgs{
+		ContainerID: testContainerID,
+		Netns:       netnsPath,
+		IfName:      "test-dummy",
+		StdinData:   []byte(conf),
+	}
+
+	// host-device DEL will fail (no host-device binary next to the test
+	// binary) — that's expected and non-fatal, matching production when the
+	// move-based side effect doesn't apply anyway.
+	if err := cmdDel(args); err != nil {
+		t.Fatalf("cmdDel returned error = %v, want nil", err)
+	}
+
+	if gw := defaultRouteVia(t, netnsPath); gw != nil {
+		t.Fatalf("default route gateway = %v, want nil (flushed by DEL)", gw)
+	}
+}
+
 func mustParseCIDR(t *testing.T, cidr string) *net.IPNet {
 	t.Helper()
 	_, ipnet, err := net.ParseCIDR(cidr)

internal/cni/netns.go

@@ -167,6 +167,75 @@ func isDefaultRouteDst(dst *net.IPNet) bool {
 	return ones == 0
 }
 
+// flushGuestNetnsConfig removes any IP addresses and default routes
+// configured on ifName inside the container netns, without deleting the
+// link itself. No-op if ifName is not present.
+//
+// DEL normally relies on hostDevice DEL moving the guest veth end back out
+// of the container netns to flush this state as a side effect: the kernel
+// strips a link's addresses/routes when it genuinely crosses a namespace
+// boundary. But that move is a no-op — and so triggers no such flush — when
+// the "container" netns is the same namespace the link already lives in
+// (e.g. a hostNetwork pod with a Multus secondary attachment, where
+// args.Netns resolves to the host's own root netns rather than a distinct
+// per-sandbox namespace). Left unflushed, that state survives indefinitely
+// (there is no ephemeral sandbox netns to tear down and reclaim it), and the
+// next ADD on that same interface fails with "file exists" trying to add a
+// default route that never went away. Calling this unconditionally in DEL,
+// ahead of hostDevice DEL, makes cleanup reliable regardless of whether the
+// move-triggered flush fires.
+func flushGuestNetnsConfig(netnsPath, ifName string) error {
+	containerNS, err := ns.GetNS(netnsPath)
+	if err != nil {
+		return fmt.Errorf("get container netns %q: %w", netnsPath, err)
+	}
+	defer containerNS.Close() //nolint:errcheck // netns close on teardown
+
+	return containerNS.Do(func(_ ns.NetNS) error {
+		handle, err := netlink.NewHandle()
+		if err != nil {
+			return fmt.Errorf("create netlink handle: %w", err)
+		}
+		defer handle.Close() //nolint:errcheck // netlink cleanup on teardown
+
+		link, err := handle.LinkByName(ifName)
+		if err != nil {
+			return nil // interface not present — nothing to flush
+		}
+
+		for _, family := range []int{netlink.FAMILY_V6, netlink.FAMILY_V4} {
+			routes, err := handle.RouteList(link, family)
+			if err != nil {
+				return fmt.Errorf("list routes on %q: %w", ifName, err)
+			}
+			for _, route := range routes {
+				if !isDefaultRouteDst(route.Dst) {
+					continue
+				}
+				if err := handle.RouteDel(&route); err != nil {
+					return fmt.Errorf("delete default route on %q: %w", ifName, err)
+				}
+			}
+
+			addrs, err := handle.AddrList(link, family)
+			if err != nil {
+				return fmt.Errorf("list addresses on %q: %w", ifName, err)
+			}
+			for _, addr := range addrs {
+				if addr.IP.IsLinkLocalUnicast() {
+					continue
+				}
+				if err := handle.AddrDel(link, &addr); err != nil {
+					return fmt.Errorf("delete address %s on %q: %w", addr.IPNet, ifName, err)
+				}
+			}
+		}
+
+		slog.Debug("netns: flushed guest interface addresses/routes", "ifName", ifName, "netns", netnsPath)
+		return nil
+	})
+}
+
 // readGuestInterface reads the MAC and MTU of the guest veth endpoint
 // inside the container network namespace.
 func readGuestInterface(netnsPath, ifName string) (string, int, error) {

internal/cni/netns_test.go

@@ -362,3 +362,106 @@ func TestConfigureInterfaceInNetnsConflictingGatewayErrors(t *testing.T) {
 		t.Fatalf("default route gateway = %v, want unchanged %v", gw, otherGateway)
 	}
 }
+
+// ---- flushGuestNetnsConfig ---------------------------------------------
+
+// TestFlushGuestNetnsConfigRemovesAddressAndRoute verifies that a configured
+// address and default route are both removed, while the link itself
+// survives (unlike cleanupContainerNetns, which deletes the whole
+// interface).
+func TestFlushGuestNetnsConfigRemovesAddressAndRoute(t *testing.T) {
+	netnsPath, cleanup := createTestNetnsWithDummy(t)
+	defer cleanup()
+
+	if err := configureInterfaceInNetns(netnsPath, "test-dummy", testIPNet, testGateway, nil, nil); err != nil {
+		t.Fatalf("configureInterfaceInNetns: %v", err)
+	}
+
+	if err := flushGuestNetnsConfig(netnsPath, "test-dummy"); err != nil {
+		t.Fatalf("flushGuestNetnsConfig: %v", err)
+	}
+
+	if gw := defaultRouteVia(t, netnsPath); gw != nil {
+		t.Fatalf("default route gateway = %v, want nil (flushed)", gw)
+	}
+
+	nsObj, err := ns.GetNS(netnsPath)
+	if err != nil {
+		t.Fatalf("open netns %q: %v", netnsPath, err)
+	}
+	defer nsObj.Close() //nolint:errcheck // best-effort cleanup
+
+	err = nsObj.Do(func(_ ns.NetNS) error {
+		handle, err := netlink.NewHandle()
+		if err != nil {
+			return err
+		}
+		defer handle.Close() //nolint:errcheck // netlink cleanup on teardown
+
+		link, err := handle.LinkByName("test-dummy")
+		if err != nil {
+			return fmt.Errorf("link should still exist after flush: %w", err)
+		}
+		addrs, err := handle.AddrList(link, netlink.FAMILY_V6)
+		if err != nil {
+			return err
+		}
+		for _, a := range addrs {
+			if a.Equal(netlink.Addr{IPNet: testIPNet}) {
+				return fmt.Errorf("address %s still present after flush", testIPNet)
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestFlushGuestNetnsConfigNonExistentInterface verifies the flush is a
+// no-op (not an error) when the named interface isn't present — the case
+// where host-device DEL never got as far as moving anything in.
+func TestFlushGuestNetnsConfigNonExistentInterface(t *testing.T) {
+	netnsPath, cleanup := createTestNetnsWithDummy(t)
+	defer cleanup()
+
+	if err := flushGuestNetnsConfig(netnsPath, "does-not-exist"); err != nil {
+		t.Fatalf("expected nil for non-existent interface, got: %v", err)
+	}
+}
+
+// TestFlushGuestNetnsConfigThenRetryAddSucceeds reproduces the production
+// incident: a hostNetwork pod's Multus secondary attachment resolves
+// args.Netns to the same namespace the guest link already lives in, so
+// host-device DEL's cross-namespace move — and the kernel's implicit
+// address/route flush that comes with a *real* namespace change — never
+// happens. Without an explicit flush, the leftover default route wedges the
+// next ADD with "file exists" even though the link's own idempotency check
+// (TestConfigureInterfaceInNetnsAddTwiceIsIdempotent) only covers a
+// same-link retry, not one where cleanup ran in between but the flush
+// silently no-opped.
+func TestFlushGuestNetnsConfigThenRetryAddSucceeds(t *testing.T) {
+	netnsPath, cleanup := createTestNetnsWithDummy(t)
+	defer cleanup()
+
+	if err := configureInterfaceInNetns(netnsPath, "test-dummy", testIPNet, testGateway, nil, nil); err != nil {
+		t.Fatalf("first configureInterfaceInNetns: %v", err)
+	}
+
+	// Simulate DEL for the same-namespace (hostNetwork) case: the
+	// move-based flush hostDevice DEL relies on doesn't fire, so DEL's only
+	// effective cleanup is the explicit flush.
+	if err := flushGuestNetnsConfig(netnsPath, "test-dummy"); err != nil {
+		t.Fatalf("flushGuestNetnsConfig: %v", err)
+	}
+
+	// The retried ADD must now succeed against the same link.
+	if err := configureInterfaceInNetns(netnsPath, "test-dummy", testIPNet, testGateway, nil, nil); err != nil {
+		t.Fatalf("configureInterfaceInNetns after flush: %v", err)
+	}
+
+	gw := defaultRouteVia(t, netnsPath)
+	if gw == nil || !gw.Equal(testGateway) {
+		t.Fatalf("default route gateway = %v, want %v", gw, testGateway)
+	}
+}

internal/cni/ops_del.go

@@ -40,24 +40,35 @@ func cmdDel(args *skel.CmdArgs) error {
 		}
 	}
 
+	// Explicitly flush the address/default-route galactic-cni's IPAM step
+	// installed on the guest interface, ahead of host-device delegation.
+	// hostDevice DEL's move of the guest veth end back out of the container
+	// netns normally flushes this as a side effect of crossing a namespace
+	// boundary, but that side effect never fires when args.Netns is the same
+	// namespace the link already lives in (e.g. a hostNetwork pod with a
+	// Multus secondary attachment) — the move is then a no-op, and the
+	// leftover route survives indefinitely since there's no ephemeral
+	// sandbox netns to reclaim it, wedging the next ADD with "file exists".
+	// Only applies to veth mode; tap mode has no guest-side netns config.
+	if pluginConf.InterfaceType == interfaceTypeVeth {
+		if err := flushGuestNetnsConfig(args.Netns, args.IfName); err != nil {
+			slog.Warn("DEL: failed to flush guest interface address/route, may still be in the netns",
+				"err", err, "containerID", args.ContainerID, "netns", args.Netns)
+		}
+	}
+
 	// Forward DEL to host-device delegated plugin (CNI spec §4). This moves
-	// the guest veth end back out of the container netns, which as a side
-	// effect flushes the addresses/routes galactic-cni's IPAM step installed
-	// on it — the primary mechanism for cleaning those up. Only applies to
-	// veth mode; tap mode has no host-device delegation.
+	// the guest veth end back out of the container netns and restores its
+	// original (host-side) name. Only applies to veth mode; tap mode has no
+	// host-device delegation.
 	//
 	// DEL must always return success per the CNI spec, so an error here
 	// (e.g. the device was never moved into the netns because ADD failed
 	// before reaching that step, or the netns is already gone) is logged
-	// rather than propagated. A logged failure here is the signal to look
-	// for: it means the route/address were NOT flushed via this path and may
-	// still be sitting in the container netns for whatever picks up
-	// args.Netns next — see addAddrIfMissing/addDefaultRouteIfMissing in
-	// netns.go, which is what makes a subsequent ADD retry against that
-	// leftover state safe instead of failing with "file exists".
+	// rather than propagated.
 	if pluginConf.InterfaceType == interfaceTypeVeth {
 		if err := hostDevice("DEL", args, pluginConf); err != nil {
-			slog.Warn("DEL: host-device DEL failed, guest interface (and any route/address) may still be in the netns",
+			slog.Warn("DEL: host-device DEL failed, guest interface may still be in the netns",
 				"err", err, "containerID", args.ContainerID, "netns", args.Netns)
 		}
 	}